I know it’s not trendy to re-hash old java programming discussions – I should be discussing some new amazing functional language, however I’m in a grumpy mood. I have a beef with what appears to be the dominant trend of using setter-based dependency injection. I’m an old-skool dependency inject-er – I started with picocontainer when Spring was unheard of (well at least by me). What I particularly like was the way it changed the way I designed my java code. (For the spring-kiddies, PicoContainer *only* supported constructor injection)
A common complaint I hear about constructor injection is “I don’t like having so many constructor arguments”. Yup this crossed my mind when I first came across it too – then it dawned on me: perhaps having too many dependencies injected into a class is a code smell? How many other classes should this class be collaborating with? Am I missing an abstraction in my model? Many times I’ve looked at a class and simply by looking at it’s constructor signature I’ve had a nagging doubt about it’s hygiene.
A big reason I prefer constructor injection is that I can ensure that when an object is constructed is ready to do it’s job. I do not have to concern myself with whether the class has been ‘wired’ correctly in configuration and that all dependencies have been met. If I introduce a new dependency, I can find all places in the code (and pretty quickly in configuration) where I’ve caused a breakage.
However it’s now 2009 and I’m still (occasionally) writing java, but pretty regularly I’m in a team that has settled on setter injection as a standard. It’s the default option in the Rod Johnson scriptures I guess. I also believe in most cases it’s better to be consistent than argue what some see as a fine point, so again I fall into line and am forced to use setter injection. But I do want to bitch about it.
How about this test setup code:
WidgetGetter widgetGetter = new WidgetGetter(); widgetGetter.setHttpClient(new HttpClient()); WeeResourceFactory resourceFactory = new WeeResourceFactory(); resourceFactory.setWidgetGetter(widgetGetter); resourceFactory.setSomeParameter("the quick brown fox"); FooRepository repo = new FooRepository(); repo.setBarFactory(new BarFactory()); repo.setResourceFactory(resourceFactory);
WidgetGetter widgetGetter = new WidgetGetter(new HttpClient()); WeeResourceFactory resourceFactory = new WeeResourceFactory(widgetGetter, "the quick brown fox"); FooRepository repo = new FooRepository(resourceFactory, new BarFactory());
I’m also into configuration in code rather than in XML, but I worry I’ll get burned at the stake if I bring THAT up again.
13 thoughts on “Setter injection sucks”
I couldn’t agree more I hate setter injection, it requires more unnecessary code and the dreaded NullPointerException at runtime because you forgot to wire something up.
On my current project the ‘standard’ is to use setters, at least I have IntelliJ to generate them for me.
I’d have to say the version using setters is better.
Sure the constructor means less lines of code, but all those lines of setters express far more clearly what is actually happening.
When creating the WeeResourceFactory, you pass it “the quick brown fox”. If you had a few String parameters, someone looking at the code would be hard pressed to know what each one means.
Instead, by using setSomeParameter, we can see that “the quick brown fox” is “some parameter” (more meaninful names would make this more obvious).
How do you feel about ctors throwing exceptions?
Yes! I agree completely. When people complain about too many constructor arguments’ making the code ugly, I tell them Dependency Injection doesn’t cause ugliness, it only exposes it. The class already had all these dependencies, they were just hidden.
Setter injection is evil.
If you use Spring for DI and your Unit Test class extends SpringTestCase, you should be able to have spring inject those dependencies for you from an applicationContext.xml file.
I try to use constructor injection for dependencies that the class cannot function without and setter injection for optional dependencies that the class can do without.
Yep, totally agree with your complaints about setter injection. I hadn’t thought about the code smell angle from constructor injection before, but it is definitely something I will keep in the back of my mind from now on.
I started out doing setter injection, but when I finally realized that objects should have everything they need in order to work before you can use them, I never looked back. Constructor injection all the way. Now if I could just get my team from defining every single object in our code base in or applicationContext.xml files. grumble, grumble, grumble.
I agree that its better to make an object’s instantiation and configuration appear atomic to clients.
But c’tor injection has problems too:
1. unless you compile with debug symbols on, the names of the injected params get erased to types, so you can’t inject by name. I wish that would change, maybe it has already in practice, cause everyone leaves debug on nowadays.
2. Sometimes its easier to break up _instatiation_ from _configuration_ of an object, because different pieces do each job.
AFAIK, thats how Spring 2.x does DI into domain objects created by foreign frameworks (eg hibernate). It uses an aspect that intercepts after the ‘new’ operation, to apply config via setter injection. Its seems a bit more invasive to completely replace the ‘new’ call using AOP.
3. It must bind against an exact concrete type. You cant substitute a subclass.
PS Got a broken link on your main page, the “more” link on this blog post.
PPS Re: “some new amazing functional language”: Scala absolutely and utterly rocks. Thoughtworks seems conspicuously quiet in the Scala community right now. Come on board!!
I agree that setter injection is undesirable. It encourages the creation of objects in invalid states, and it often hides intent.
Don’t worry about your preference for configuration in code as opposed to XML. Not everyone will agree with you, but lots of smart people do.
When they want to create objects, they use constructors. That’s what constructors are for.
I couldn’t agree with you more. Constructors put the object in the correct state at the time of construction. If you need to use setters before you’ve used the object then you should be beefing up your constructors.
Completely agree in general.
It seems (to me, anyway) that a more appropriate pattern to use for situations where you are likely to have different implementations is the factory pattern. When I run my code in a debugger to find an issue I can actually see what’s happening and what sub-class my code is actually using (and why). When I get a NPE from bad configuration (assuming the factory is configurable) I actually know immediately where to go look. No such luck with Spring 🙁
Good point. Seems like a reasonable way to help bridge the differences between the two approaches, but keep code (and particularly intent) sane and understandable.
The desire for names is understandable, but the fault is not in the concept of constructor injection but rather in the Java language (I assume you’re talking about Java) itself. If you use Scala, then one of the myriad benefits you get is the ability to pass parameters by name when you so desire. So now you can write nice, tight code to invoke constructors and be sure that all of the dependencies are being satisfied, while still seeing the name of every dependency.
hi and thanks, this web site seriously aided me with a writing theme for my school class at MSU