Search code examples
.netunit-testingreplicationencapsulation

Encapsulation or no code replication in .Net C# unit tests


In a .Net C# project I had to create unit test for a function. In the preparation of that unit tests, I need to create some object with some specific properties. I already have a function, in my base code, which create that object with those specific properties. The problem is that function is private. In this scenario what is the most correct thing to do? Creating a copy of the function in order to maintain the function private and maintain the encapsulation rule. Or should I change the function to public in order to maintain the no code replication rule?

The code:

I have a Service1 that has a private function to transform an instance of a Class1DB to an instance Class1 class. The Class1DB is used to manage all the data from a data base. This service have a function to get a class1 instance by a specific id.

public static class Service1
{

    public static Class1 GetClass1ById(int Id)
    {

        Class1DB class1DB = GetClass1DBById(Id);


        return GetClass1FromClass1DB(class1DB)
    }

    private static Class1 GetClass1FromClass1DB(Class1DB class1DB)
    {
        Class1 class1 = new Class1();
        class1.Id = class1DB.Id;

        ...

        return class1;

    }

To create unit tests for the function Service1.GetClass1ById() I need to use the GetClass1FromClass1DB() so that I can create an instance of Class1DB in the data base and then get an instance of Class1 based in the created Class1DB. The assert of the test will be the comparison of the instance of Class1 from the GetClass1FromClass1DB() and the instance of Class1 from the Service1.GetClass1ById();

    [TestMethod]
    public void Service1_GetClass1ById_test()
    {
        Class1DB class1DB = CreateNewClass1DBInDB();

        Class1 class1 = Service1.GetClass1FromClass1DB(class1DB);

        Class1 class1ToTest = Service1.Service1.GetClass1ById(class1DB.Id);

        Assert.IsTrue(IsClass1InstancesEquals(class1, class1ToTest))
    }

The IsClass1InstancesEquals function just compare the properties of the two classes and return the result of the comparison.


Solution

  • TL;DR;

    You need to make an assertion on each field of the returned classed, rather than just comparing two classes, if you do the latter you're actually assuming the function called GetClass1FromClass1DB, which is a private implementation detail of the class, is correct, when in fact that's not what you're testing for.


    There's a couple of things here that are code smells and hint at a bad design if you're doing TDD.

    First of this is not a unit test, it's an integration test since in order to test your service you're forced to create an object in the database. That hints there's a hard dependency on a concrete implementation detail, mainly the method GetClass1DBById.

    Since that method (I assume) brings back a "Class1DB" from the database you should replace that with an interface that can then be mocked.

    public interface IClass1Store
    {
      Class1DB GetClass1DBById(int id);
    }
    

    which you can then mock and setup on your test

    var class1Store = new Mock<IClass1Store>();
    var serviceToTest = new Service1(class1Store.Object);
    

    With that you've removed a hidden, non-explicit dependency and substituted it with an explicit dependency (that'll make your code more maintainable).

    That will allow you to generate the Class1DB for the test and mock the database access.

    var myDbClass1 = GenerateTestClass1DB();
    var class1Store = new Mock<IClass1Store>();
    class1Store.Setup(x => x.GetClass1DBById(myDbClass1.Id)).Returns(myDbClass1);
    var serviceToTest = new Service1(class1Store.Object);
    

    And then, finally you test on the individual properties that you want to test, that is, this:

    Assert.IsTrue(IsClass1InstancesEquals(class1, class1ToTest))
    

    is not a valida assertion. The purpose of the test is: "if I have this information in the database I will get back a class which this and this values". To be able to test that you need to define what those values are, rather than just comparing the classes with a call to a private function.

    You want to do something like:

    Assert.IsTrue(4, class1.Id); 
    Assert.IsTrue("MyValue", class1.OtherPropery);
    

    If you do that, you will be assuming GetClass1FromClass1DB and that's an assumption you don't want to make.