C# – Should the classes have separate constructors just for unit testing

cunit testing

I like to write classes with two constructors: a primary constructor used in production code, and a default constructor just for unit tests. I do this because the the primary constructor creates other concrete dependencies that consume external resources. I can't do all that in a unit test.

So my classes look like this:

public class DoesSomething : BaseClass
{
    private Foo _thingINeed;
    private Bar _otherThingINeed;
    private ExternalResource _another;

    public DoesSomething()
    {
        // Empty constructor for unit tests
    }

    public DoesSomething(string someUrl, string someThingElse, string blarg)
    {
         _thingINeed = new Foo(someUrl);
         _otherThingINeed = Foo.CreateBar(blarg);
         _another = BlargFactory.MakeBlarg(_thingINeed, _otherThingINeed.GetConfigurationValue("important");
    }
}

This is a pattern I follow with many of my classes so that I can write unit tests for them. I always include the comment in the default constructor so others can tell what it's for. Is this a good way to write testable classes or is there a better approach?

It was suggested that this might be a duplicate of "Should I have one constructor with injected dependencies and another that instantiates them." It's not. That question is about a different way to create the dependencies. When I use the default unit test constructor the dependencies don't get created at all. Then, I make most methods public and they don't use the dependencies. Those are the ones I test.

Best Answer

Never, ever create separate code paths for unit testing. Doing so defeats the purpose of unit testing because you're testing one code path in your unit tests and executing a different one in production.

For some reason that seems insufficient because I'm just repeating parts of your question back to you. You already know you're executing different code in your unit tests. The comment you put in your constructor says that. It's like asking if it's appropriate to take someone else's cell phone because it belongs to them, not to you, and you want to have it.

Would an argument from consensus help? Don't do that because no one else does it. Someone who knows better will be shocked. Even worse, someone who doesn't know better might learn from it, which would be bad, because... I feel like I'm still going in circles. But hopefully it carries some weight that other people say you shouldn't do that.

Perhaps I could show you that there is a better way: Dependency inversion and dependency injection. Dependency inversion means that you depend on abstractions (like interfaces) instead of concrete implementations. That's impossible if your class is creating its own dependencies, which is where dependency injection comes in.

Instead of creating instances of dependencies, you define abstractions that describe how your class needs to use them. For example, if ExternalResource is something you download files from, you could create an interface like this:

public interface IFileStorage
{
    byte[] GetFile(string filePath);
}

Then you create your class like this:

public class DoesSomething : BaseClass
{
    private readonly IFileStorage _fileStorage;

    public DoesSomething(IFileStorage fileStorage)
    {
        _fileStorage = fileStorage;
    }
}

Now you don't need a separate constructor. Your unit tests can call the real constructor and pass in a mock or a test double, which means an implementation of IFileStorage that returns exactly what you want it to. That way you can write tests for how your class behaves if the file contains this or that or nothing at all, without using an actual concrete implementation of IFileStorage.

For example, what if you want to see how your class behaves if IFileStorage returns an empty byte array? You could use a framework like Moq or just do this:

public class FileStorageThatReturnsEmptyArray : IFileStorage
{
    public byte[] GetFile(string filePath)
    {
        return new byte[] {};
    }
}

Then your test subject looks like this:

var subject = new DoesSomething(new FileStorageThatReturnsEmptyArray());

Isn't that better? Your unit tests are worth something because they test your class end-to-end, allowing it to behave exactly as it does in production.

True, in production you'll have a concrete implementation of IFileStorage instead of a test double. But the point is that to DoesSomething it's all the same. One IFileStorage is the same as another.

Is that a contradiction, since I just said that you should test the "real" code, but now I'm recommending using mocks? No. The question was about unit tests, which means that you wanted to test DoesSomething, so you would use mocks to test that class. You could write additional unit tests for other classes or integration tests to test them together. In the scope of this answer, if you're going to test DoesSomething then the point is that you wouldn't want to deceive yourself by testing a non-production method of that class, thinking that it's the same as testing the production method. ("Production" and "Non-production" methods aren't even a thing.)

I hope that showing how other developers solved this problem helps.

Related Topic