Are we using IoC effectively?

Question

So my company uses Castle Windsor IoC container, but in a way that feels "off":

• All the data types are registered in code, not the config file.
• All data types are hard-coded to use one interface implementation. In fact, for nearly all given interfaces, there is and will only ever be one implementation.
• All registered data types have a default constructor, so Windsor doesn't instantiate an object graph for any registered types.

The people who designed the system insist the IoC container makes the system better. We have 1200+ public classes, so its a big system, the kind where you'd expect to find a framework like Windsor. But I'm still skeptical.

Is my company using IoC effectively? Is there an advantage to new'ing objects with Windsor than new'ing objects with the new keyword?

Gyrostatic

Answer 1

The short answer: No, your company isn't using DI effectively.

The slightly longer answer: The main problem is that all classes have default constructors. When that's the case, then how do you resolve dependencies?

Either your constructors have hard-coded dependencies like this:

public Foo()
{
    IBar bar = new Bar();
}

In which case you can't vary the dependencies without recompiling that application.

Even worse, they may use the Static Service Locator anti-pattern:

public Foo()
{
    IBar bar = Container.Resolve<IBar>();
}

A DI Container should resolve the entire dependency graph in the application's Composition Root and get out of the way.

In general it's better to use Convention over Configuration by using a heuristic approach in code to configure the container. XML configuration should be reserved for those few cases where you need to be able to reconfigure dependencies without recompiling, which should only be a small subset. In short, I see no inherent problem with configuring the container in code. In fact, it's the preferred approach.

Having only one implementation per interface is not a problem either. It's a start, and if the application is truly loosely coupled, it's a constantly open window of opportunity. Sooner or later you will be very likely to introduce alternative implementations, but you can best do this if the interfaces are already in place and the entire code base adhere to the Liskov Substitution Principle.

Trilley

+1, +answer: looks like you nailed it. At first I thought IoC was adding a needless layer of indirection to code, when in fact the real problem is that we're not actually managing our dependencies correctly. And yes, we do in fact have a static class called ServiceLocator, and its basically being used as a replacement for constructors. Your article has been forwarded around, and I'm hoping to see some improvements in the not too distant future. Much appreciated :) – Gyrostatic

Mark is too modest to mention it, but you may also check out the book he's writing on this very subject: manning.com/seemann – Brandes

Answer 2

All the data types are registered in code, not the config file.

By data types I presume you mean the bindings. For example, IFoo should be bound with Foo. If this is the case, that's fine. In fact, this is much better in many peoples eyes (mine included) than specifying bindings via XML. The natural benefit here is that at compile time checks are made. You won't be programming in XML as they say.

All data types are hard-coded to use one interface implementation. In fact, for nearly all given interfaces, there is and will only ever be one implementation.

Are you sure? If so, this could be a smell. However you've stated it's a large application. If there is any unit testing (or automated testing) going on, having an interface per implementation is perfectly valid.

Is my company using IoC effectively? Is there an advantage to new'ing objects with Windsor than new'ing objects with the new keyword?

Yes, from what I can gather. For any form of automated testing, creating a dependency within an object is a bad thing to do. By using an IOC container the complex wiring of classes can be removed when it comes to actually connecting the production code together. This simplifies the application, and allows you as the developer to focus on features, and other tasks, rather than ensuring the whole thing actually builds.

Also, by inverting your control, making changes to modular parts of the application is very easy. For example, imagine if you wished to switch a module for a better version. With IOC containers you simply modify the one or two lines of bindings. Without IOC, you'd have to manually traverse through the application and ensure it was correctly wired internally.

This is a good question that relates to your last point, on why you should use IOC over straight forward manual methods.

Cassella

Yes, we have lots and lots of tests. And actually from the looks of it, we use IoC for a handful of types in some places, and manual dependency injection in others. So right now our code is suffering from inconsistent handling of dependencies -- ideally I'd like to use Windsor for most or all important instantiations, or gut it entirely. I'm not sure how much we use it in testing, I'll have to get back to you on that :) – Gyrostatic

One or the other in my opinion. The fact you're using an IOC container you should at least use it to it's fulliest. With regards using an IOC container for testing, you can, but you don't have to. In fact I don't use IOC for unit testing code, favouring to explicity inject my dependencies. – Cassella

Answer 3

I'm sure others will post more detailed answers but here's what I've got:

All the data types are registered in code, not the config file.

There's a trend in (at least some) IoC/Dependency Injection frameworks to register your dependencies using code rather than XML Config. The main benefit is you know at compile time if something is wired up wrong or not. For example if you decide to add/remove a constructor argument your code will actually not compile when your configuration is out of sync (and it keeps typos from exhibiting themselves as random null reference exceptions at runtime).

All data types are hard-coded to use one interface implementation. In fact, for nearly all given interfaces, there is and will only ever be one implementation.

I'm not really sure what you mean by "one interface implementation". If you mean interfaces are downcast to objects that implement them, then that's definitely not kosher. If not then there's a benefit to coding to an interface rather to a concrete implementation. The main one is obviously having the ability to use mock objects backing said interface in unit testing. Programming to an interface (in theory) forces you to make your code to be more loosely coupled which can help testability. It also opens some some really cool design options like this one Build a generic typesafe DAO with Hibernate and Spring AOP one.

All registered data types have a default constructor, so Windsor doesn't instantiate an object graph for any registered types.

This also isn't necessarily a bad thing. I think in general the rule of thumb is that if a dependency is crucial for your class to perform it's function, then constructor injection should be used. If not (logging is probably the most overused example of this) then property injection is a more appropriate option. Although I'm not familiar with Castle Windsor, I know a least some Java IoC containers didn't support constructor inject in early versions (if someone who's more familiar with the Castle Windsor can comment on this, that'd be awesome). If Castle Windsor didn't have this feature in earlier versions, that could have also lead to the design you're seeing in your system.

As far as using IoC vs calling new everywhere, that also depends on the usage. The few projects I worked on that relied heavily on IoC (Spring in Java and Autofac in .NET), wiring up all of my dependencies in once place allowed us to fairly quickly and painlessly experiment with object hierarchies and composition.

Sericin

+1 on compile time benefit of config vs. xml - I missed that out in my comment – Almazan

Castle Windsor does support constructor injection – Bearable

@Frank Schwieterman I looked up whether it supports constructor injection now =). I meant has it supported constructor injection from the begging or was it a feature that was added in a later version. – Sericin

.ctor injection was supported since dinosaurs. – Brandes

Answer 4

Your first two issues are not real problems (Finglas describes why so).

I would consider "All registered data types have a default constructor" as a code smell (assuming the class is loading dependencies directly from your IoC container). When this happens its either 1) an application entry point where it is appropriate to get the object from your Ioc container (as you probably need to initialize that Ioc container anyhow), or 2) a place you could be using a factory instead.

Having a default constructor fill in dependencies is discouraged. The reason being you're duplicating the knowledge of how to create the class. Ideally each class should not even have to know where your IoC container is - thats the job of your application entry point.

It sounds like the IoC usage at your company is not bad overall.

Bearable