Mocking introduces handling in production code

Question

Assuming an IReader interface, an implementation of the IReader interface ReaderImplementation, and a class ReaderConsumer that consumes and processes data from the reader.

public interface IReader
{
     object Read()
}

Implementation

public class ReaderImplementation
{
    ...
    public object Read()
    {
        ...
    }
}

Consumer:

public class ReaderConsumer()
{
    public string location

    // constructor
    public ReaderConsumer()
    {
        ...
    }

    // read some data
    public object ReadData()
    {
        IReader reader = new ReaderImplementation(this.location)
        data = reader.Read()
        ...
        return processedData    
    }
}

For testing ReaderConsumer and the processing I use a mock of IReader. So ReaderConsumer becomes:

public class ReaderConsumer()
{
    private IReader reader = null

    public string location

    // constructor
    public ReaderConsumer()
    {
        ...
    }

    // mock constructor
    public ReaderConsumer(IReader reader)
    {
        this.reader = reader
    }

    // read some data
    public object ReadData()
    {
        try
        {
            if(this.reader == null)
            {
                 this.reader = new ReaderImplementation(this.location)
            }

            data = reader.Read()
            ...
            return processedData    
        }
        finally
        {
            this.reader = null
        }
    }
}

In this solution mocking introduces an if sentence for the production code since only the mocking constructor supplies an instances of the interface.

During writing this I realise that the try-finally block is somewhat unrelated since it is there to handle the user changing the location during application run time.

Overall it feels smelly, how might it be handled better?

Answer 1

Instead of initializing the reader from your method, move this line

{
    this.reader = new ReaderImplementation(this.location)
}

Into the default parameterless constructor.

public ReaderConsumer()
{
    this.reader = new ReaderImplementation(this.location)
}

public ReaderConsumer(IReader reader)
{
    this.reader = reader
}

There is no such thing as a "mock constructor", if your class has a dependency that it requires in order to work, then the constructor should either be provided that thing, or create it.

Answer 2

You only need the single constructor:

public class ReaderConsumer()
{
    private IReader reader = null

    public string location

    // constructor
    public ReaderConsumer(IReader reader)
    {
        this.reader = reader;
    }

in your production code:

var rc = new ReaderConsumer(new ReaderImplementation(0));

in your test:

var rc = new ReaderConsumer(new MockImplementation(0));

Answer 3

Look into Dependency Injection and Inversion of Control

Both Ewan and RubberDuck have excellent answers. But I wanted to mention another area to look into which is Dependency Injection (DI) and Inversion of Control (IoC). Both of these approaches moves the problem you're experiencing into a framework/library so that you don't have to worry about it.

Your example is simple, and is quickly dispensed with, but, inevitably you're going to build upon it and you'll end up with either tons of constructors or initialization routines that look like:

var foo = new Foo(new Bar(new Baz(), new Quz()), new Foo2());

With DI/IoC you use a library which allows you to spell out the rules for matching interfaces to implementations and then you just say "Give me a Foo" and it works out how to wire it all up.

There are lots of very friendly IoC Containers (as they are called) out there, and I'm going to recommend one to look at, but, please explore, as there are very many fine choices.

A simple one to start with is:

http://www.ninject.org/

Here's a list to explore:

http://www.hanselman.com/blog/ListOfNETDependencyInjectionContainersIOC.aspx