Last week Jimmy Bogard posted why he believed the dependency injection pattern appearing in the NerdDinner application was flawed.  I want to take the opportunity to say the pattern in NerdDinner really isn't that bad and I want to explain why.  Before I go on I will say that I entirely agree with Jimmy's thoughts on the pattern for my own code and you won't find the anti-pattern Jimmy describes in my code.

So why isn't the pattern that bad? I see Dependency Injection as generally having at least two major benefits:

• Testability
• Flexibility (most often achieved by using an IoC container)

The pattern under scrutiny in NerdDinner accounts for testability, of which I am a firm believer in, but lacks in flexibility.  If I had to rank the two items above, I would rank testability above flexibility.  Testability is a core tenet on which I stand for with the code I write.

Explain Again Why NerdDinner is "Flawed"

To recap here is the code from Jimmy's post:

               1:
              public
              class SearchController
: Controller {

               2:  

               3: IDinnerRepository
dinnerRepository;

               4:  

               5:
              //
            

               6:
              //
Dependency Injection enabled constructors
            

               7:  

               8:
              public SearchController()

               9: : this(new DinnerRepository())
{

               10: }

               11:  

               12:
              public SearchController(IDinnerRepository
repository) {

               13: dinnerRepository
= repository;

               14: }

The first constructor, when called, "news up" a new DinnerRepository() and passes it into another constructor on the same object which is generally considered "poor man's dependency injection".  This pattern is considered bad (this an anti-pattern) because some, of which Jimmy is one, believe you shouldn't "new up" new objects this way, choosing instead to allow a container to inject the primal dependency.  The problem I have with this view is that for many applications a container is overkill.  Many applications will only have one concrete implementation of a class, so in many ways this constructor and the object creation could be seen as providing a sensible default to the application, allowing the consumer to create an instance without worry about what else the object needs.

But You Said You Don't Write Your Code that Way?

Absolutely correct, I don't write my code this way.  Why? I already have a container set up in my application and so to not use it would be foolish.  We also have a complex application/domain, sometimes with multiple concrete implementations of an interface.

I used to write code this way using this pattern. Really. I don't feel ashamed to say so. The reason we adopted the dependency injection (DI) pattern in the application where this overloaded constructor "anti-pattern" was used was because DI allows for testability, and as I have previously mentioned, this "anti-pattern" doesn't stand in the way of testing.

I further believe that most developers must make a certain progression on their own.  We can write and share our experiences but in the end every developer out there must some day be convinced in their own time. \They must come to the conclusion on their own, like I did, that using "poor man's injection" will work for many many scenarios but will get ugly fast in others.

What's Really Wrong with "Poor Man's Dependency Injection"?

For simple applications/domain, typically not much (again, my opinion).  For complex domain the issue you will run into is maintainability/flexibility.  If you get to a complex domain and you use the pattern from NerdDinner you will someday have:

               1:
              public SearchController(new SearchService(new SearchLogService(new Logger()), new SearchRepository(new SessionBuilder()), new SearchResultFormatter(new FormatProvider())))

Ugly for sure.  This is why it's generally considered an "Anti-Pattern".  However for applications where the controller is the service-layer and controllers therefore use repositories, I would not be too concerned with the use of this pattern.

While Jimmy did a great job fixing the application, you first have to believe the application needed fixing.  Which of course, depends on context.  How big/complex is your application/domain?  Jimmy's application/domain is sufficiently complex as is mine, yours may not be.  For NerdDinner I believe the pattern was a suitable choice.

Shhh...don't tell anyone but we don't unit test all of our code.  We're striving to get all developers and managers on board with unit testing but we're not there yet.  Despite our delinquency in writing tests one thing we do try our very best to do is keep mistakes from happening again.  If we find a bug in the system, before fixing it, we go and write a unit test for that problem.  We verify the test fails, the go and write the code that fixes it.  This recognizes that bugs will appear in the system, but once fixed we'll insulate ourselves from that bug happening again.

We had one such case last week.  We were getting a yellow-screen error after a bunch of commits by developers.  The error stemmed from a component in Windsor that did not have all of it's dependencies met.  In other words, we registered a component which relied on another component which was not registered.  We already had written a test to compile and test our Windsor configuration, however that test was not mature enough and needed some tweaking. 

In our application we leverage the new ASP.Net MVC.  We also take advantage of the ability to leverage an IoC container, Windsor, to create controller instances when a web request comes in.  When the application starts, we register all of the controllers in the application into Windsor using reflection.  Therefore our Windsor configuration at runtime is made up of two things, the XML configuration and the controllers which are registered on startup.  The "bug" here was that one of our controllers had a dependency on it that was not registered in Windsor.  When you ran the test by itself, the test would pass, since it was only loading up the components from the Windsor configuration.

Wanting to protect us in the future, we acknowledged there was something missing from our test and set out to find it.  To create the failing test was relatively simple.  All that we needed to do replicate in our test what we were doing at runtime.  Therefore we needed to register all implementers of IController, just like we do on application startup.  Below I've

Before:

               1: IWindsorContainer
container = new WindsorContainer("windsor.config.xml");

               2:  

               3:
              foreach (var
handler in container.Kernel.GetAssignableHandlers(typeof(object)))

               4: {

               5: Console.WriteLine("{0}\n\t-
{1}", handler.ComponentModel.Service, handler.ComponentModel.Implementation);

               6: container.Resolve(handler.ComponentModel.Service);

               7: }

After:

               1: IWindsorContainer
container = new WindsorContainer("windsor.config.xml");

               2:  

               3: var
assm = new HomeController(null).GetType().Assembly;

               4:
              //add
all controllers
            

               5:
              foreach (Type
type in assm.GetTypes())

               6: {

               7:
              if (typeof(IController).IsAssignableFrom(type)
&& !type.IsAbstract)

               8: {

               9: container.AddComponent(type.Name.ToLower(),
type);

               10: }

               11: }

               12:  

               13:
              foreach (var
handler in container.Kernel.GetAssignableHandlers(typeof(object)))

               14: {

               15: Console.WriteLine("{0}\n\t-
{1}", handler.ComponentModel.Service, handler.ComponentModel.Implementation);

               16: container.Resolve(handler.ComponentModel.Service);

               17: }

Now that this test is in place, I am much more confident that we won't see the error we were seeing in product again, we've got a test which warns us if something isn't quite right.  Our initial test didn't quite get it all right.  Mistakes/bugs in code will happen, hopefully few and far between, but when a bug does show up, go write a test that verifies the bug before going off an fixing it.  Then make the test pass, which should fix the bug.