Saturday, May 28, 2016

Constructor Injection >> Field Injection

The Guice documentation for injection starts with "Constructor Injection" above the fold. When I was working at Google, I went for a really long time before I even realized that field injection was supported in Guice, as it was so rare to see in actual code.

But when I started doing Spring again (having previously used it in the pre-annotation-based days), I was somewhat horrified to see that field injection via @Autowired seems to be the normal practice in modern Spring code. (For the record, the Spring documentation for annotation-based injection actually kicks off with setter injection, but most code you'll see around the web uses fields.)

In general, I think field injection is poor practice. Let me list the reasons why.

  1. Injected fields cannot be made final. Well, that's not 100% true as Guice will try a setAccessible(true) trick, but in general you don't want to go there. What you do want to do is have immutable classes where possible. (Basic Effective Java stuff.) And the things you @Inject (services and the like), you almost certainly want to be final. Field injection confounds this.
  2. Injected fields are harder to debug. With constructor injection, you can set a breakpoint in the constructor and see precisely what got bound and what the DI framework is handing your object, at the precise time it's happening. Field injection is less predictable and harder to observe in this regard.
  3. Injected fields make unit test setup much more complicated. For me, this is the big one. Let's say you want to unit test with mocks/fakes/stubs. (Certainly you do, as one of the main reasons you are using DI in the first places it to unlock better testability.) In that case, if you are using injected fields, you have two choices for unit test setup, and they are both bad:
    1. Make your fields settable after construction -- either by expanding their scope to package or public, or providing package/public setters into which your tests can push test instances. (Read: break encapsulation.) But at least the fields aren't final thanks to #1, so you've got that going for you, which is nice .
    2. Fire up the DI container in your unit test and have the container wire up the mocks/fakes/stubs for you. This is problematic for two reasons. First, DI containers do a ton of reflection, and this can slow your unit test start by a couple of orders of magnitude in a non-trivial application. (30-second unit-test startup, anyone?) Second, the container will probably find and instantiate not just your desired class-under-test and its dependencies, but a bunch of other things that you probably don't want to initialize, or alternatively have to worry about re-binding to fast, harmless or no-op implementations. From here, there is a slippery slope down to an insanely-complicated parallel DI configuration just for the unit tests, and you don't want to maintain that.
The main argument I've heard in favor of field injection is that it reduces boilerplate code by a couple of lines. Doesn't seem worth it to me. Use constructor injection instead.

No comments: