The Danger Of Mislead Solutions: Constructor Over-Injection

Recently Jeffrey Palermo blogged about something he calls "constructor over-injection anti-pattern". This post caused a couple of reactions, in particular Mark Seemann's rebuttal to be mentioned. Mark Seemann already did a nice job arguing against the tight coupling in Jeffrey's proposed solution of (taken from the follow-up)

IOrderShipper shipper = new OrderShipperFactory().GetDefault();

to lazily resolve the IOrderShipper reference. Although I agree with Mark, that those lines are as wrong as possible and introduce an even worse smell than the one Jeffrey tried to cure, I disagree with him, that the problem here is the dependency on a concrete type and that we're dealing with the servicelocator antipattern. The real problem with this solution is that it is only a variant of the singleton pattern. Misko Hevery wrote a nice post, why the singleton pattern is a really bad idea.

What's The Problem?

Let's take a step back and look at Jeffery's original problem. He started with the problem, that it took a long time to instantiate the OrderProcessor because of the constructor-dependency on an IOrderShipper. From the fact, that IOrderShipper is not used by OrderProcessor in 100% of the cases, he draws the conclusion, that IOrderShipper should not be a mandatory argument to the OrderProcessor, thus effectivly turning IOrderShipper into an optional dependency of OrderProcessor.

Is IOrderShipper Really Optional?

What does making IOrderShipper an optional dependency of OrderProcessor buy us? Of course OrderProcessor get's initialized much faster now. But this comes at a price. By removing the dependency from the constructor (or passing it in via .NET 4.0's Lazy<> as some suggested), we are giving up some important advantages.

For one, we won't get any notice if there's anything wrong with the IOrderShipper until it is actually needed. There might be an important resource like a database not available, a configuration error or whatever. Only when the first user enters a valid order, this user will experience an error. This violates the fail fast & early principle.

If everything is configured correctly, this first user also will experience a surprising delay while the system is processing his order, as he has to wait until the IOrderShipper is available. This breaks the principle of least surprise.

The Real Problem: IOrderShipper

After all, it is to expect, that the majority of our users will enter a valid order. The conclusion, that IOrderShipper is an optional dependency, is simply plain wrong when looking at the real world scenario. Thus making IOrderShipper an optional dependency of OrderProcessor due to performance issues when instantiating the object graph is just curing a symptom. It is not about healing the real problem: the terrible performance of instantiating the IOrderShipper.

The lesson I learned from this exercise: always carefully evaluate, that you're drawing the right conclusions from your analysis. It is sometimes too tempting to take the easy path or be mislead and in doing so introduce even bigger problems in the long run than the one currently at hand.

As a sidenote, I absolutely agree with Jeffrey, that IoC containers are just tools and your application's design should not rely on your container's features. After all, non-invasiveness is one of the fundamentals of IoC.

So Constructor Over-Injection Is Not An Issue?

In his second post, Jeffrey introduces even more dependencies into OrderProcessor to proof his point. I have to admit, that services classes sometimes tend to grow and serve as a sort of "feature attractors". In my next post I will look into this problem and provide some thoughts on why this is a problem and how to solve it.