Which objects to mock when doing TDD

Question

When creating methods, should every object instantiated inside that method be passed in as a parameter so that those objects can be mocked in our unit tests?

We have a lot of methods here at work that have no associated unit tests and upon writing tests retrospectively; we find that there are quite a lot of objects instantiated inside these methods.

One of our options is to refactor our current methods into more unit like methods and reduce the number of responsibilities per methods. This could be quite a lengthy process but most certainly be a large benefit to us in the future.

What do you think? Should all objects instantiated inside a method be passed in as parameters?

Answer 1

Maybe not all objects, but the more objects you inject into your unit, the better your separation of concerns tend to become, so I would definitely recommend that you move in that direction.

You don't have to pass all objects as method parameters. It is often a better design to inject collaborators into classes via Constructor Injection. This keeps your interfaces clean while your implementations can import the collaborators it needs.

Let's say that your original implementation looked like this:

public class Foo
{
    public Ploeh DoStuff(Fnaah f)
    {
        var bar = new Bar();
        return bar.DoIt(f);
    }
}

This could be changed to look like this:

public class Foo
{
    private readonly IBar bar;

    public Foo(IBar bar)
    {
        this.bar = bar;
    }

    public Ploeh DoStuff(Fnaah f)
    {
        return this.bar.DoIt(f);
    }
}

Notice that I changed bar from an instance of Bar to an instance of IBar, thus decoupling Foo from the specific implementation of IBar. Such a refactoring tend to make your unit tests simpler to write and maintain, since you can now vary the implementations of Foo and Bar independently.

Answer 2

The first part is a bit of a loaded question. It's like asking "When running over pedestrians in my car, should I have both hands on the wheel?"

A method that is instantiating lots of other objects is almost certainly doing too much. A class with lots of those methods is probably not following the single responsibility principle.

However, one of the key ways to make your code testable is to use IoC (inversion of control), where a class's dependencies (or a method's) are passed to it, rather than the class asking for them. This makes it much easier to test, as you can pass in mocks.

So the short answer is "Yes", pass in your dependencies and look at a good dependency injection component. The long answer is "Yes, but don't do that". DI frameworks will probably force you to pass in dependencies to objects rather than methods, and you'll find you want to make sure you limit those dependencies - which is a good thing.

And certainly, refactoring to reduce dependencies is good. Shortening your methods to do one thing is almost never bad. I would strongly agree this is a long term gain, as long as you can afford the short term costs.