Dependency Inversion expands the API, results in unnecessary tests

Question

This question has bothered me for a few days, and it feels like several practices contradict each other.

Example

Iteration 1

public class FooDao : IFooDao
{
    private IFooConnection fooConnection;
    private IBarConnection barConnection;

    public FooDao(IFooConnection fooConnection, IBarConnection barConnection)
    {
        this.fooConnection = fooConnection;
        this.barConnection = barConnection;
    }

    public Foo GetFoo(int id)
    {
        Foo foo = fooConection.get(id);
        Bar bar = barConnection.get(foo);
        foo.bar = bar;
        return foo;
    }
}

Now, when testing this, I would fake IFooConnection and IBarConnection, and use Dependency Injection (DI) when instantiating FooDao.

I can change the implementation, without changing the functionality.

Iteration 2

public class FooDao : IFooDao
{
    private IFooBuilder fooBuilder;

    public FooDao(IFooConnection fooConnection, IBarConnection barConnection)
    {
        this.fooBuilder = new FooBuilder(fooConnection, barConnection);
    }

    public Foo GetFoo(int id)
    {
        return fooBuilder.Build(id);
    }
}

Now, I won't write this builder, but imagine it does the same thing FooDao did before. This is just a refactoring, so obviously, this doesn't change the functionality, and so, my test still passes.

IFooBuilder is Internal, since it only exists to do work for the library, i.e it isn't a part of the API.

The only problem is, I no longer comply to Dependency Inversion. If I rewrote this to fix that problem, it might look like this.

Iteration 3

public class FooDao : IFooDao
{
    private IFooBuilder fooBuilder;

    public FooDao(IFooBuilder fooBuilder)
    {
        this.fooBuilder = fooBuilder;
    }

    public Foo GetFoo(int id)
    {
        return fooBuilder.Build(id);
    }
}

This should do it. I have changed the constructor, so my test needs to change to support this (or the other way around), this isn't my issue though.

For this to work with DI, FooBuilder and IFooBuilder need to be public. This means that I should write a test for FooBuilder, since it suddenly became part of my library's API. My problem is, clients of the library should only be using it through my intended API design, IFooDao, and my tests acts as clients. If I don't follow Dependency Inversion my tests and API are more clean.

In other words, all I care about as the client, or as the test, is to get the correct Foo, not how it is built.

Solutions

• Should I simply not care, write the tests for FooBuilder even though it is only public to please DI? - Supports iteration 3

• Should I realise that expanding the API is a downside of Dependency Inversion, and be very clear about why I chose not to comply to it here? - Supports iteration 2

• Do I put too much emphasis on having a clean API? - Supports iteration 3

EDIT: I want to make it clear, that my problem is not "How to test internals?", rather it is something like "Can I keep it internal, and still comply to DIP, and should I?".

Answer 1

I'm going to share some experiences/observations of various code bases I've seen. N.B. I'm not advocating any approach in particular, just sharing with you where I see such code is going:

Should I simply not care, write the tests for FooBuilder even though it is only public to please DI

Before DI and automated testing, the accepted wisdom was that something should be restricted to the scope required. Nowadays however, it isn't uncommon to see methods that could be left internal made public for ease of testing (since this usually happens in another assembly). N.B. there is a directive to expose internal methods but this more or less amounts to the same thing.

Should I realise that expanding the API is a downside of Dependency Inversion, and be very clear about why I chose not to comply to it here?

DI does undoubtedly incur an overhead with additional code.

Do I put too much emphasis on having a clean API

Since DI frameworks do introduce additional code, I've noticed a shift towards code that while written in the DI style does not use DI per se i.e. the D from SOLID.

In summary:

1. Access modifiers are sometimes loosened to simplify testing

2. DI does introduce additional code

In the light of 1 & 2, some developers are of the view that this is too much of a sacrifice and so simply elect to depend on abstractions initially with the option to retro-fit DI later.

Answer 2

I'd go with:

Should I realise that expanding the API is a downside of Dependency Inversion, and be very clear about why I chose not to comply to it here? - Supports iteration 2

However, in The S.O.L.I.D. Principles of OO and Agile Design (a at 1:08:55) Uncle Bob says that his rule about dependency injection is don't inject everything, you inject only at strategic locations. (He also mentions that the topics of dependency inversion and dependency injection are the same).

That is, dependency inversion is not meant to be applied on all class dependencies in a program. You should do it at strategic locations (when it pays of (or might pay of)).

Answer 3

I don't particularly buy this notion that you have to expose private methods (by whatever means) in order to perform suitable testing. I would also suggest that your client facing API is not the same as your object's public method e.g. here's your public interface:

interface IFooDao {
   public Foo GetFoo(int id);
}

and there's no mention of your FooBuilder

In your original question, I would take the third route, using your FooBuilder. I would perhaps test your FooDao using a mock, thus concentrating on your FooDao functionality (you can always inject a real builder if it's easier) and I would have separate tests around your FooBuilder.

(I'm surprised mocking hasn't been referred to in this question/answer - it goes hand-in-hand with testing DI/IoC-patterned solutions)

Re. your comment:

As stated, the builder provides no new functionality, so I should not need to test it, however, DIP makes it part of the API, which forces me to test it.

I don't think this widens the API you present to your clients. You can restrict the API your clients use via interfaces (if you so wish). I would also suggest that your tests shouldn't just surround your public-facing components. They should embrace all the classes (implementations) that support that API underneath. e.g. here's the REST API for the system I'm currently working on:

(in pseudo-code)

void doSomeCalculation(json : CalculationRequestObject);

I have one API method, and 1,000's of tests - the vast majority of which are around the objects supporting this.

Answer 4

Why do you want to follow the DI in this case if not for testing purposes? If not only your public API, but also your tests are really cleaner without a IFooBuilder parameter (as you wrote), it does not make sense to introduce such a class. DI is not an end in itself, it should help you to make your code more testable. If you do not want to write automated tests for that particular level of abstraction, don't apply DI at that level.

However, lets assume for a moment you want to apply DI for allowing to create unit tests for the FooDao constructor in isolation without the building process, but still do not want a FooDao(IFooBuilder fooBuilder) constructor in your public API, only a constructor FooDao(IFooConnection fooConnection, IBarConnection barConnection). Then provide both, the former as "internal", the latter as "public":

public class FooDao : IFooDao
{
    private IFooBuilder fooBuilder;

    // only for testing purposes!    
    internal FooDao(IFooBuilder fooBuilder)
    {
        this.fooBuilder = fooBuilder;
    }

    // the public API
    public FooDao(IFooConnection fooConnection, IBarConnection barConnection)
    {
        this.fooBuilder = new FooBuilder(fooConnection, barConnection);
    }    

    public Foo GetFoo(int id)
    {
        return fooBuilder.Build(id);
    }
}

Now you can keep FooBuilder and IFooBuilder internal, and apply the InternalsVisibleTo to make them accessible by unit tests.