Guice Injection - Field or Constructor?
I was just starting to look at the the task of creating views in Vaadin 7, using Guice.
This has been made a lot easier by the new Vaadin architecture, but it prompted me to look again at some Guice
best practices, and specifically which type of injection to use.
The clarity and simplicity of field injection is always so appealing ...
Which Injection?
Generally the choice is between constructor and field injection (method injection is usually only required under specific circumstances, although some espouse the virtue of setter injection).
Field injection
is very easy to write, and the intent of the code very clear - but does not result in immutability. Note this comment from the Guice team:
"Never inject final fields; the JVM doesn't guarantee that the injected value will be visible to all threads."
Constructor injection will always happen before field inject, which would mean constructor code would not have access to field values if field injection was used.
The only point I would disagree with is the suggestion that field injection is not testable. Provided you use Guice in your test harness, as the
sample app does using
Mycila, testing injected fields is entirely possible.
Constructor injection
provides immutability, but the code is a lot less clear when there are multiple injections, especially when injecting providers. As the Guice team describe it, the code is cumbersome. A change to a sub-class constructor with a lot of injections is a real pain and can lead to mistakes.
Originally I used constructor injection for immutability, and experienced some lengthy and cumbersome code as a consequence. The clarity of field injection is so tempting (which is why it tends to be used for code examples) ... that I decided to re-assess my thinking.
So which to go for?
Ultimately it is up to you of course, and there are use cases for all of them.
After reading quite a few posts, with many opinions, I concluded that constructor injection is generally the best. The main reasons:
- immutability. As something which is generally considered good practice, this is fairly strong argument in its own right ... final fields cannot be injected reliably as mentioned above, although Guice will not raise any errors.
- consistency. Although not essential, it may be helpful to default to one technique. You can usually use constructor injection where field injection is used. (There are exceptions, for example in servlets which are required to have a parameterless constructor). The reverse is less often true, if, for example, the constructor code needs to access the field values.
- clarity. If a constructor gets too long, I should probably refactor the class anyway, and avoiding deep class hierarchies has other benefits.
- risk. Not a huge risk admittedly, but it would be easy to forget why an injected field is not marked private, and automatically generate a setter for it. Of course then the setter could be called elsewhere and displace the injected value. That's fine if it is intentional, but it would be fairly easy to do accidentally.
... but the decision is not a religious one, so if I come across a scenario where field injection (or method injection) would do a better job, then I'd use it. The default for me, though, is constructor injection, so I will update the
sample app to reflect that.
But
It would be really good if someone could come up with a DI solution which combines the best of constructor and field injection ... :-)
