18

I've used various IoC containers (Castle.Windsor, Autofac, MEF, etc) for .Net in a number of projects. I have found they tend to be frequently abused and encourage a number of bad practices.

Are there any established practices for IoC container use, particularly when providing a platform/framework? My aim as a framework writer is to make code as simple and as easy to use as possible. I'd rather write one line of code to construct an object than ten or even just two.

For example, a couple of code smells that I've noticed and don't have good suggestions to:

  1. Large number of parameters (>5) for constructors. Creating services tends to be complex; all of the dependencies are injected via the constructor - despite the fact that the components are rarely optional (except for maybe in testing).

  2. Lack of private and internal classes; this one may be a specific limitation of using C# and Silverlight, but I'm interested in how it is solved. It's difficult to tell what a frameworks interface is if all the classes are public; it allows me access to private parts that I probably shouldnt touch.

  3. Coupling the object lifecycle to the IoC container. It is often difficult to manually construct the dependencies required to create objects. Object lifecycle is too often managed by the IoC framework. I've seen projects where most classes are registered as Singletons. You get a lack of explicit control and are also forced to manage the internals (it relates to the above point, all classes are public and you have to inject them).

For example, .Net framework has many static methods. such as, DateTime.UtcNow. Many times I have seen this wrapped and injected as a construction parameter.

Depending on concrete implementation makes my code hard to test. Injecting a dependency makes my code hard to use - particularly if the class has many parameters.

How do I provide both a testable interface, as well as one that is easy to use? What are the best practices?

Dave Hillier
  • 3,940
  • 1
  • 25
  • 37
  • 1
    What bad practices do they encourage? As for private classes, you shouldn't need to have very many of them if you have good encapsulation; for one thing, they're much harder to test. – Aaronaught Sep 01 '12 at 23:09
  • Err, 1 and 2 are the bad practices. Large constructors and not using encapsulation to establish a minimal interface? Also, I said private and internal (use internals visible to, to test). I have never used a framework that forces me to use a particular IoC container. – Dave Hillier Sep 01 '12 at 23:16
  • 2
    Is it possible that perhaps a sign of needing many parameters for your class constructors is itself the code smell and the DIP is just making this more obvious? – dreza Sep 01 '12 at 23:23
  • I certainly think that does not help - but even then, should I have a constructor that accepts an interface if the normal usage never changes it? For example - many parts of the .Net framework have static methods - should you depend on them if it makes you code untestable. If Injecting dependencies sensible if you're building a framework? – Dave Hillier Sep 01 '12 at 23:27
  • IoC containers don't encourage #1 and #2. #1 is just bad code and #2 isn't a problem in good APIs. #3 is intentional, it's just singleton scoping that's wrong in some cases (i.e. database connections) - but sloppy/inexperienced developers manage to do that just as easily *without* IoC containers (I see new examples of singleton abuse every time I look at Stack Overflow, and it almost never involves IoC). – Aaronaught Sep 01 '12 at 23:35
  • I would disagree, I have seen them encourage it over a number of projects. You've been lucky, I guess people have done it the right way.... What is that right way? – Dave Hillier Sep 01 '12 at 23:36
  • Static methods are also not inherently untestable. They just shouldn't have any other dependencies. Those are just utility methods, which is fine. It's when you have static methods depending on other static methods or static global/singleton instances that you start running into testability problems. – Aaronaught Sep 01 '12 at 23:36
  • @Aaronaught have you written a framework that utilises an IoC container, but does not force it upon the client? If so please detail in an answer how you achieved it without falling to the bad practices I listed. – Dave Hillier Sep 01 '12 at 23:39
  • How is it that you have "seen them encourage it" - how are you establishing a connection to IoC containers as opposed to just bad coding practices? Well-run shops tend to do automated dependency analysis in the commit stage and fail the build or issue warnings if the efferent coupling is too high. But constructor over-injection is not limited to IoC containers; I've seen it plenty of times (or worse, *Property* over-injection) in plain old code. At least they're both better than the alternative which is global instances and/or new-ing up these objects at random times. – Aaronaught Sep 01 '12 at 23:39
  • @Aaronaught please read again. I have no problem testing static methods. I *do* have a problem testing methods with dependencies on static fields or properties. – Dave Hillier Sep 01 '12 at 23:41
  • I'd really like to show you some of the frameworks I've written but sadly they are not my copyright. They are aspect-orientation frameworks for caching and performance measurements, they both rely heavily on DI but are not coupled to any IoC container and most classes depend on only 1 or 2 other interfaces (3 or 4 in extreme cases, like the runtimes). It is way easier to test that way. Maybe we'll open-source them at some point. All I can say is that DI frameworks don't encourage bad dependency management, they just make it more obvious when it's in use. – Aaronaught Sep 01 '12 at 23:43
  • I disagree, hence my question. I've not found any references to support your opinion, so I'll leave my question. I wish I worked at a "Well-run shop"/no true scotsman. – Dave Hillier Sep 01 '12 at 23:46
  • Please point me to one popular open-source .NET library or framework that has unit tests and show me where it either doesn't use DI where it could or uses some DI anti-pattern like over-injection. Specific examples would make for a better discussion. – Aaronaught Sep 01 '12 at 23:48
  • 1. This is a question based upon my experience and what I have found, not a statement of universal truth. 2. In the Open Source framework I have used they do not use these IoC containers and I consider that a good thing. – Dave Hillier Sep 01 '12 at 23:50
  • 3
    Using a *specific IoC container* in a framework generally isn't good practice. Using *dependency injection* **is** a good practice. Are you aware of the difference? – Aaronaught Sep 02 '12 at 00:12
  • 1
    @Aaronaught (back from the dead). Using, "dependency injection is a good practice" is false. Dependency Injection is a tool that should be used only when appropriate. Using Dependency Injection all the time is a bad practice. – Dave Hillier Oct 15 '13 at 12:31

1 Answers1

27

The only legitimate dependency injection anti-pattern that I'm aware of is the Service Locator pattern, which is an anti-pattern when a DI framework is used for it.

All of the other so-called DI anti-patterns that I've heard about, here or elsewhere, are just slightly more specific cases of general OO/software design anti-patterns. For instance:

  • Constructor over-injection is a violation of the Single Responsibility Principle. Too many constructor arguments indicates too many dependencies; too many dependencies indicates that the class is trying to do too much. Usually this error correlates with other code smells, such as unusually long or ambiguous ("manager") class names. Static analysis tools can easily detect excessive afferent/efferent coupling.

  • Injection of data, as opposed to behaviour, is a subtype of the poltergeist anti-pattern, with the 'geist in this case being the container. If a class needs to be aware of the current date and time, you don't inject a DateTime, which is data; instead, you inject an abstraction over the system clock (I usually call mine ISystemClock, although I think there's a more general one in the SystemWrappers project). This is not only correct for DI; it is absolutely essential for testability, so that you can test time-varying functions without needing to actually wait on them.

  • Declaring every life cycle as Singleton is, to me, a perfect example of cargo cult programming and to a lesser degree the colloquially-named "object cesspool". I've seen more singleton abuse than I care to remember, and very little of it involves DI.

  • Another common error is implementation-specific interface types (with strange names like IOracleRepository) done just to be able to register it in the container. This is in and of itself a violation of the Dependency Inversion Principle (just because it's an interface, does not mean it's truly abstract) and often also includes interface bloat which violates the Interface Segregation Principle.

  • The last error I usually see is the "optional dependency", which they did in NerdDinner. In other words, there is a constructor that accepts dependency injection, but also another constructor that uses a "default" implementation. This also violates the DIP and tends to lead to LSP violations as well, as developers, over time, start making assumptions around the default implementation, and/or start new-ing up instances using the default constructor.

As the old saying goes, you can write FORTRAN in any language. Dependency Injection isn't a silver bullet that will prevent developers from screwing up their dependency management, but it does prevent a number of common errors/anti-patterns:

...and so on.

Obviously you don't want to design a framework to depend on a specific IoC container implementation, like Unity or AutoFac. That is, once again, violating the DIP. But if you find yourself even thinking about doing something like that, then you must have already made several design errors, because Dependency Injection is a general-purpose dependency-management technique and is not tied to the concept of an IoC container.

Anything can construct a dependency tree; maybe it's an IoC container, maybe it's a unit test with a bunch of mocks, maybe it's a test driver supplying dummy data. Your framework shouldn't care, and most frameworks I've seen don't care, but they still make heavy use of dependency injection so that it can be easily integrated into the end user's IoC container of choice.

DI isn't rocket science. Just try to avoid new and static except when there's a compelling reason to use them, such as a utility method that has no external dependencies, or a utility class that could not possibly have any purpose outside the framework (interop wrappers and dictionary keys are common examples of this).

Many of the problems with IoC frameworks come up when developers are first learning how to use them, and instead of actually changing the way they handle dependencies and abstractions to fit the IoC model, instead try to manipulate the IoC container to meet the expectations of their old coding style, which would often involve high coupling and low cohesion. Bad code is bad code, whether it uses DI techniques or not.

Aaronaught
  • 44,005
  • 10
  • 92
  • 126
  • I have a few questions. For a library that is distributed on NuGet, I cannot depend on a IoC system as that's done by the application using the library, not by the library itself. In many cases, the user of the library really doesn't care about its internal dependencies at all. Is there a problem with having a default constructor initiating the internal dependencies, and a longer constructor for unit testing and/or customized implementations? Also you say to avoid "new". Does this apply for things like StringBuilder, DateTime and TimeSpan? – Etienne Charland Feb 12 '19 at 13:41