Unit testing and dependency injection with deeply

Question

Assume a legacy class and method structure like below

public class Foo
{
    public void Frob(int a, int b)
    {
        if (a == 1)
        {
            if (b == 1)
            {
                // does something
            }
            else
            {
                if (b == 2)
                {
                    Bar bar = new Bar();
                    bar.Blah(a, b);
                }
            }
        }
        else
        {
            // does something
        }
    }
}

public class Bar
{
    public void Blah(int a, int b)
    {
        if (a == 0)
        {
            // does something
        }
        else
        {
            if (b == 0)
            {
                // does something
            }
            else
            {
                Baz baz = new Baz();
                baz.Save(a, b);
            }
        }
    }
}

public class Baz
{
    public void Save(int a, int b)
    {
        // saves data to file, database, whatever
    }
}

And then assume management issues a nebulous mandate to perform unit testing for every new thing we do, be it an added feature, modified requirement, or bug fix.

I may be a stickler for literal interpretation, but I think the phrase "unit testing" means something. It does not mean, for example, that given inputs of 1 and 2 that the unit test of Foo.Frob should succeed only if 1 and 2 are saved to a database. Based on what I've read, I believe it ultimately means based on inputs of 1 and 2, Frob invoked Bar.Blah. Whether or not Bar.Blah did what it is supposed to do is not my immediate concern. If I'm concerned with testing the entire process, I believe there's another term for that, right? Functional testing? Scenario testing? Whatever. Correct me if I'm being too rigid, please!

Sticking with my rigid interpretation for the moment, let's assume I want to try to utilize dependency injection, with one benefit being that I can mock away my classes so that I can, for example, not persist my test data to a database or file or whatever the case may be. In this case, Foo.Frob needs IBar, IBar needs IBaz, IBaz may need a database. Where are these dependencies to be injected? Into Foo? Or does Foo merely need IBar, and then Foo is responsible for creating an instance of IBaz?

When you get into a nested structure such as this, you can quickly see there could be multiple dependencies necessary. What is the preferred or accepted method of performing such injection?

Answer 1

When you have trouble with a deeply nested hierarchy it just means you aren't injecting enough dependencies.

The issue here is that we have Baz and it looks like you need to pass Baz to Foo who passes it to Bar who finally calls a method on it. This seems like a lot of work and kinda useless...

What you should do is pass Baz as the parameter of the Bar object constructor. Bar should then be passed to the constructor of the Foo object. Foo should never touch or even know about the existence of Baz. Only Bar cares about Baz. When testing Foo, you would use another implementation of the Bar interface. This implementation probably does nothing but record that the fact that Blah was called. It does not need to consider the existence of Baz.

You are probably thinking something like this:

class Foo
{
    Foo(Baz baz)
    {
         bar = new Bar(baz);
    }

    Frob()
    {
         bar.Blah()
    }
}

class Bar
{
     Bar(Baz baz);

     void blah()
     {
          baz.biz();
     }
}

You should do something like this:

class Foo
{
    Foo(Bar bar);

    Frob()
    {
         bar.Blah()
    }
}

class Bar
{
     Bar(Baz baz);

     void blah()
     {
          baz.biz();
     }
}

If you do it correctly, each object should only need to deal with the objects it directly interacts with.

In your actual code, you construct the objects on the fly. To do that you'll just need to pass instances of BarFactory and BazFactory to do construct the objects when required. The basic principle remains the same.

Answer 2

the kind of test you described in the first part of your post (when you try all the parts together) it is usually defined as integration test. As a good practice in your solution you should have either a unit test project and an integration test project. In order to inject dependecies in your code the first and most important rule is to code using interfaces. Assumed this, let's say your class contains an interface as a member and you want to inject/mock it: you can either expose it as a property or pass the implementation using the class constructor. I prefer to use properties to expose dependencies, this way the constructor don't become too verbose. I suggest you to use NUnit or MBunit as a testing framework and Moq as a mocking framework (more clear in it's outputs than Rhino mocks) Here's the documentation with a some examples on how to mock with Moq http://code.google.com/p/moq/wiki/QuickStart

Hope it helps

Answer 3

Let us start with your last question. Where are the dependencies injected: A common approach is to use constructor injection (as described by Fowler). So Foo is injected with an IBar in the constructor. The concrete implementation of IBar, Bar in turn has an IBaz injected into its constructor. And finally the IBaz implementation (Baz) has an IDatabase (or whatever) injected. If you use a DI framework such as Castle Project, you would simply ask the DI container to resolve an instance of Foo for you. It will then use whatever you have configured to determine which implementation of IBar you are using. If it determines that your implementation of IBar is Bar it will then determine which implementation of IBaz you are using, etc.

What this approach gives you, is that you can test each of the concrete implementations in isolation, and just check that it invokes the (mocked) abstraction correctly.

To comment on your concerns about being too rigid etc, the only thing I can say is that in my opinion you are choosing the right path. That said, management might be in for a surprise when the actual cost of implementing all those tests becomes apparent to them.

Hope this helps.

Answer 4

Sounds like there's a bit of a struggle here to:

1. deal with legacy code
2. continue to write maintainable code
3. test your code (continuation of 2 really)
4. and also, I assume, release something

Management's use of 'unit tests' can only by determined by asking them, but here's my 2c on what might be a good idea here in regards to all 4 issues above.

I think it's important to both test that Frob invoked Bar.Blah and that Bar.Blah did what it's supposed to do. Granted these are different tests, but in order to release bug-free (or as-few-bugs-as-possible) software, you really need to have unit tests (Frob invoked Bar.Blah) as well as integration tests (Bar.Blah did what it's supposed to do). It would be great if you could unit test Bar.Blah too but if you don't expect that to change then it might not be too useful.

Certainly going forward you'll want to add unit tests every time you find a bug, preferably before fixing. This way you can ensure the test breaks before fixing and then the fix causes the test to pass.

You don't want to spend all day refactoring or rewriting your code base so you need to be judicious in how you go about dealing with dependencies. In the example you gave, inside Foo you might be best off promoting Bar to an internal property and setting up the project to make internals visible to your test project (using the InternalsVisibleTo attribute in AssemblyInfo.cs). The default constructor of Bar could set the property to new Bar(). Your test can set it to some subclass of Bar used for testing. Or a stub. I think that will cut down on the amount of changes you'll have to make to make this thing testable going forward.

And of course you don't need to do any refactoring of a class until you make some other changes to that class.

Answer 5

I don't think there is one "preferred" method for addressing this, but one of your main concerns seems to be that with dependency injection, when you create Foo, you need to also create Baz which might be unnecessary. One simple way around this is for Bar not to depend directly on IBaz but on a Lazy<IBaz> or a Func<IBaz>, allowing your IoC container to create an instance of Bar without immediately creating Baz.

For example:

public interface IBar
{
    void Blah(int a, int b);
}

public interface IBaz
{
    void Save(int a, int b);
}

public class Foo
{
    Func<IBar> getBar;
    public Foo(Func<IBar> getBar)
    {
        this.getBar = getBar;
    }

    public void Frob(int a, int b)
    {
        if (a == 1)
        {
            if (b == 1)
            {
                // does something
            }
            else
            {
                if (b == 2)
                {                        
                    getBar().Blah(a, b);
                }
            }
        }
        else
        {
            // does something
        }
    }
}



public class Bar : IBar
{
    Func<IBaz> getBaz;

    public Bar(Func<IBaz> getBaz)
    {
        this.getBaz = getBaz;
    }

    public void Blah(int a, int b)
    {
        if (a == 0)
        {
            // does something
        }
        else
        {
            if (b == 0)
            {
                // does something
            }
            else
            {
                getBaz().Save(a, b);
            }
        }
    }
}

public class Baz: IBaz
{
    public void Save(int a, int b)
    {
        // saves data to file, database, whatever
    }
}

Answer 6

I'd say you're right about unit testing, it should cover a fairly small 'unit' of code, although exactly how much is up for debate. However, if it touches the database, that's almost certainly not a unit test - I'd call that an integration test.

Of course, it could be that 'management' don't really care about such things and would be quite happy with integration tests! They're still perfectly valid tests, and probably easier for you to add in, although don't necessarily lead to better design like unit tests tend to.

But yes, inject your IBaz into your IBar when that gets created, and inject your IBar into your Foo. This can be done in the constructor or a setter. Constructor is (IMO) better as it leads to only valid objects being created. One option you can do (known as poor man's DI) is to overload the constructor, so you can pass in an IBar for testing, and create a Bar in the parameterless constructor used in code. You lose the good design benefits, but worth considering.

When you've worked all that out, try an IoC container such as Ninject, which may make your life easier.

(Also consider tools such as TypeMock or Moles, which can mock things without an interface - but bear in mind that's cheating and you won't get an improved design, so should be a last resort).