Dependency Injection is only MOSTLY Good

Miracle Max: He probably needs a dependency on a collabator.
Inigo Montoya: Use dependency injection. It's good.
Miracle Max: Whoo-hoo-hoo, look who knows so much. It just so happens that dependency injection is only MOSTLY good. There's a big difference between mostly good and all good. Mostly good is slightly bad.

---

We know that we have to manage dependencies if we want maintainable code, which implies that there are good dependencies and bad dependencies. Well, it's just as easy to inject bad dependencies as good ones unless you're paying attention. But because we're injecting them, it is sometimes not as easily noticed even if you are paying attention. In part because the dependencies become implicit. In part because they are hiding beneath the banner of "dependency injection", which we (erroneously) believe to be inherently a good thing. They're not. They're just mostly good.

Here's an example from the project I'm working on now. We have an ApplicationContext[?] from which we can get a SaveOperationFactory[?], from which we can get a SaveThingOperation[?]. So the production code inside a ThingPresenter[?] might look like this:

public class ThingPresenter
{
  private ApplicationContext context;
  private Thing thing;

  public ThingPresenter(ApplicationContext context)
  {
    this.context = context;
  }

  public Thing Thing
  {
    set { thing = value; }
  }

  public void SaveThing()
  {
    context.SaveOperationFactory.NewSaveThingOperation(thing).Execute();
  }
}

Our test looks like this (using a hand coded mock):

public class ThingPresenterTest
{
  [Test] public void SaveShouldDelegateToSaveThingOperation()
  {
    thingPresenter.SaveThing();
    Assert.IsTrue(saveThingOperation.executeWasCalled);
  }

Seems simple, right? Not so fast. I didn't show you the setup!

public class ThingPresenterTest
{
  [SetUp] public void SetUp()
  {
	applicationContext = new MockApplicationContext();
	thingPresenter = new ThingPresenter(applicationContext);
  }

Still seems simple, right? Not so fast, let's look at MockApplicationContext[?]:

public class MockApplicationContext : ApplicationContext
{
  private SaveOperationFactory mockSaveOperationFactory = new MockSaveOperationFactory();
  public SaveOperationFactory SaveOperationFactory { get { return mockSaveOperationFactory; }}
}

Now let's look at the MockSaveOperationFactory[?]..

public class MockSaveOperationFactory
{
  public SaveThingOperation NewSaveThingOperation
  {
    return new MockSaveThingOperation();
  }
}

OK - you get the idea by now. Here's an alternative using RhinoMocks:

public class ThingPresenterTest
{
  [Test] public void SaveShouldDelegateToSaveThingOperation()
  {
    Expect.Call(saveThingOperation.Execute());
    mocks.ReplayAll();
    thingPresenter.SaveThing();
    mocks.VerifyAll();
  }

If you're familiar w/ Rhino Mocks this should present no problem - but let's look at the setup:

public class ThingPresenterTest
{
  [SetUp] public void SetUp()
  {
	applicationContext = (ApplicationContext) mocks.DynamicMock(typeof(ApplicationContext));
	saveOperationFactory = (SaveOperationFactory) mocks.DynamicMock(typeof(SaveThSaveOperationFactory));
	saveThingOperation = (SaveThingOperation) mocks.DynamicMock(typeof(SaveThingOperation));
	Expect.Call(applicationContext.SaveOperationFactory).Return(saveOperationFactory);
	Expect.Call(saveOperationFactory.NewSaveThingOperation(null)).IgnoreArguments.Return(saveThingOperation);

	thingPresenter = new ThingPresenter(applicationContext);
  }

And that's just the setup for this one test method. There are other methods on the ThingPresenter[?] that get other factories from the ApplicationContext[?], which mean there are more mocks to setup. Look at each piece and it looks inocuous, but there are two big problems here.

One is that trying to drive this from tests means a LOT of setup before you can even get the right players in your test. If I want to add an OtherThing[?] to my application, with an OtherThingPresenter[?], I can't test the OtherThingPresenter[?] until I hook everything up in the various mocks.

The other is that debugging is, well, debugging. Chasing down execution paths through several class files, etc, etc, etc. It's painful. And it sucks. It has all the testing problems we get from Service Locator. In fact, injecting dependencies on factories is just Service Locator in disguise.

Here's an alternative. Just give the ThingPresenter[?] exactly what it needs:

public class ThingPresenterTest
{
  [SetUp] public void SetUp()
  {
    thingSaver = (ThingSaver) mocks.DynamicMock(typeof(ThingSaver));
    thingPresenter = new ThingPresenter(thingSaver);
    thing = new Thing();
    thingPresenter.Thing = thing;
  }

  [Test] public void SaveShouldDelegateToThingSaver()
  {
    Expect.Call(thingSaver.SaveThing(thing));
    mocks.ReplayAll();
    thingPresenter.SaveThing();
    mocks.VerifyAll();
  }

What you see there is everything we need for this test. We can write it just like that and implement the code without depending on anything implicit. The ThingSaver[?] can deal with getting a new SaveThingOperation[?], the code that builds real ThingPresenters[?] in production can use the ApplicationContext[?] to get what it needs to build the ThingPresenter[?], but that's just wiring which is easily tested elsewhere and we really don't need (and shouldn't need) to depend on here.

I think part of the problem is the name "Dependency Injection". It's actually not injecting dependencies at all. In fact, it's protecting us from them. But that name makes it sound like it's good to inject dependencies. Perhaps "Dependency Hiding" would better describe what we're after, just like "Data Hiding" and "Implementation Hiding" - the things that lead us toward highly decoupled, highly maintainable code.

---

!commentForm