Does this FizzBuzz code correctly follow SRP and unit-testing?

Question

This is code for FizzBuzz. I have also hosted in on my GitHub.

FizzChecker Tests

[TestClass]
public class FizzChecker_Tests
{
    IFizzChecker _fizzChecker;
    bool isFizz;

    [TestInitialize]
    public void TestInitialize()
    {
        //Arrange
        _fizzChecker = new FizzChecker();
        isFizz = default(bool);
    }

    [TestMethod]
    public void When_Devisible_By_Three_Return_True()
    {
        //Act
        isFizz = _fizzChecker.IsFizz(3);

        //Assert
        Assert.AreEqual(isFizz, true);
    }

    [TestMethod]
    public void When_Not_Devisible_By_Three_Return_False()
    {
        //Act
        isFizz = _fizzChecker.IsFizz(1);

        //Assert
        Assert.AreEqual(isFizz, false);
    }

    [TestMethod]
    public void When_Devide_By_Zero_Return_True()
    {
        //Act
        isFizz = _fizzChecker.IsFizz(0);

        //Assert
        Assert.AreEqual(isFizz, true);
    }
}

Fizz

public interface IFizzChecker
{
    bool IsFizz(int number);
}
public class FizzChecker : IFizzChecker
{
    public bool IsFizz(int number)
    {
        return (number % 3) == 0;
    }
}

Buzz tests

[TestClass]
public class BuzzChecker_Tests
{
    BuzzChecker _buzzChecker;
    bool isFizz;

    [TestInitialize]
    public void TestInitialize()
    {
        //Arrange
        _buzzChecker = new BuzzChecker();
        isFizz = default(bool);
    }

    [TestMethod]
    public void When_Devisible_By_Fize_Return_True()
    {
        //Act
        isFizz = _buzzChecker.IsBuzz(5);

        //Assert
        Assert.AreEqual(isFizz, true);
    }

    [TestMethod]
    public void When_Not_Devisible_By_Three_Return_False()
    {
        //Act
        isFizz = _buzzChecker.IsBuzz(1);

        //Assert
        Assert.AreEqual(isFizz, false);
    }

    [TestMethod]
    public void When_Devide_By_Zero_Return_True()
    {
        //Act
        isFizz = _buzzChecker.IsBuzz(0);

        //Assert
        Assert.AreEqual(isFizz, true);
    }
}

BuzzChecker

public interface IBuzzChecker
{
    bool IsBuzz(int number);
}
public class BuzzChecker : IBuzzChecker
{
    public bool IsBuzz(int number)
    {
        return (number % 5) == 0;
    }
}

I've written a test for FizzBuzzPrinter. Is it correct to mock the return results on IBuzzChecker and IFizzChecker instead of using concrete implementation? Since I think that I am testing FizzBuzzPrinter on this abstraction, not its dependency, so I mock all the dependency.

[TestClass]
public class FizzBuzzPrinter_Tests
{
    IFizzBuzzPrinter _fizzBuzzPrinter;
    Mock<IFizzChecker> _mockFizzChecker;
    Mock<IBuzzChecker> _mockBuzzChecker;

    [TestInitialize]
    public void Initialize()
    {
        _mockFizzChecker = new Mock<IFizzChecker>();
        _mockBuzzChecker = new Mock<IBuzzChecker>();
        _fizzBuzzPrinter = new FizzBuzzPrinter(_mockFizzChecker.Object, _mockBuzzChecker.Object);
    }

    [TestMethod]
    public  void When_Number_Is_In_FizzCheckerConditionOnly_Return_Fizz()
    {
        //Arrange
        int number = 3;
        _mockFizzChecker.Setup(checker => checker.IsFizz(number)).Returns(true);
        _mockBuzzChecker.Setup(checker => checker.IsBuzz(number)).Returns(false);

        //Act
        string result = _fizzBuzzPrinter.Print(number);
        Assert.AreEqual("Fizz", result);
    }

    [TestMethod]
    public void When_Number_Is_In_BuzzCheckerConditionOnly_Return_Fizz()
    {
        //Arrange
        int number = 5;
        _mockFizzChecker.Setup(checker => checker.IsFizz(number)).Returns(false);
        _mockBuzzChecker.Setup(checker => checker.IsBuzz(number)).Returns(true);

        //Act
        string result = _fizzBuzzPrinter.Print(number);
        Assert.AreEqual("Buzz", result);
    }

    [TestMethod]
    public void When_Number_Is_In_Both_BuzzChecker_And_FizzChecker_Condition_Return_FizzBuzz()
    {
        //Arrange
        int number = 15;
        _mockFizzChecker.Setup(checker => checker.IsFizz(number)).Returns(true);
        _mockBuzzChecker.Setup(checker => checker.IsBuzz(number)).Returns(true);

        //Act
        string result = _fizzBuzzPrinter.Print(number);
        Assert.AreEqual("FizzBuzz", result);
    }


    [TestMethod]
    public void When_Number_Is_Not_In_Both_BuzzChecker_And_FizzChecker_Condition_Return_Original_Number()
    {
        //Arrange
        int number = 22;
        _mockFizzChecker.Setup(checker => checker.IsFizz(number)).Returns(false);
        _mockBuzzChecker.Setup(checker => checker.IsBuzz(number)).Returns(false);

        //Act
        string result = _fizzBuzzPrinter.Print(number);
        Assert.AreEqual("22", result);
    }
}

FizzBuzzPrinter

public interface IFizzBuzzPrinter
{
    string Print(int number);
}
public class FizzBuzzPrinter : IFizzBuzzPrinter
{
    private IFizzChecker fizzChecker;
    private IBuzzChecker buzzChecker;

    public FizzBuzzPrinter(IFizzChecker fizzChecker, IBuzzChecker buzzChecker)
    {
        this.fizzChecker = fizzChecker;
        this.buzzChecker = buzzChecker;
    }

    public FizzBuzzPrinter() : this (new FizzChecker(), new BuzzChecker())
    {

    }

    public string Print(int number)
    {
        if (fizzChecker.IsFizz(number) && buzzChecker.IsBuzz(number))
        {
            return "FizzBuzz";
        }

        if (fizzChecker.IsFizz(number))
        {
            return "Fizz";
        }

        if(buzzChecker.IsBuzz(number))
        {
            return "Buzz";
        }

        return number.ToString();
    }
}

Then in the main Program, I call it like this:

class Program
{
    static void Main(string[] args)
    {
        IFizzBuzzPrinter fizzBuzzPrinter = new FizzBuzzPrinter();

        for (int i = 1; i <= 100; i++)
        {
           Console.WriteLine(fizzBuzzPrinter.Print(i));
        }

        Console.ReadKey();
    }
}

Does this follow the proper mindset of SRP and unit-testing? Is there anything wrong that needs to be improved?

Answer 1

I think you're overdoing it. SRP does not mean that every class should have only one method and every method should be a one-liner.

Instead, you should structure your code into parts that are likely to change together (though that's not the only criterion). But here, it's not really possible to guess that, because it's obvious these are not real requirements.

I think that FizzBuzz is a good problem to test that you know the basics of programming (logical way of thinking about problems) or to test that you know the basics of some programming language (ifs, functions). But because of how artificial the problem is, it's not very good for testing things like SRP or unit testing.

Answer 2

I know this is an old post, but it got bumped and I noticed this oddness:

   public string Print(int number)
    {
        if (fizzChecker.IsFizz(number) && buzzChecker.IsBuzz(number))
        {
            return "FizzBuzz";
        }

       if (fizzChecker.IsFizz(number))
        {
            return "Fizz";
        }

       if(buzzChecker.IsBuzz(number))
        {
            return "Buzz";
        }

       return number.ToString();
    }
}

I would expect a function named "Print" to print something (Probably to standard output), not return a string. Considering you have an interface for the printer, and you're printing to the Console, I would say that FizzBuzzPrinter should output directly to console. That way you can avoid doing this.

Console.WriteLine(fizzBuzzPrinter.Print(i));

And replace it with:

fizzBuzzPrinter.Print(i);

Different implementations of IFizzBuzzPrinter could Print to different outputs as necessary.

@Jamal's idea in the comments about calling Print() ToString() instead is probably a better idea though. Like @BenAaronson pointed out, different implementations would require a lot of this duplicated logic.