This dependency injection madness must end!

Or: Poor man’s dependency injection: Singleton-initialized field

As dependency injection frameworks like Spring have become central in software applications in the last few years, the conventional view is now that the Singleton pattern is evil and should never be used. Instead, relationships between objects in your system should be managed by a dependency injection container. I have started disliking the consequence of this strategy very much: All coupling in my system becomes implicit and harder to understand. I have instead reverted to using design patterns like the Singleton pattern, but with a slight twist.

The Singleton-initialized field

Here is an example of a Singleton-initialized field:

public class PersonService {

    private PersonDao personDao;

    public PersonService(PersonDao personDao) {
        this.personDao = personDao;
    }

    public PersonService() {
        SessionFactory sessionFactory = 
              MyHibernateContext.getSessionFactoryInstance();
        this.personDao = new HibernatePersonDao(sessionFactory);
    }

    public void someServiceMethod() {
       // Do something with personDao
    }

}

When I write a test for this class, the test code overrides the PersonDao by passing it to the non-default constructor.

@Test
public void shouldDoSomething() {
   PersonService service = new PersonService(mockPersonDao);
   service.someServiceMethod();
   verify(mockPersonDao).findAllPeople();
}

Compared: The dependency injection way

The now-conventional way of doing the same thing is with dependency injection:

public class PersonService {

    private PersonDao personDao;

    @Inject
    public PersonService(PersonDao personDao) {
        this.personDao = personDao;
    }

    public void someServiceMethod() {
       // Do something with personDao
    }
}

And the test will set up the dependencies using a container:

@RunWith(SpringTestRunner.class)
@Configuration("src/test/resources/test-context.xml")
public class PersonServiceTest {

   @Resource
   private PersonService service;

   @Resource
   private PersonDao mockPersonDao;

   @Test
   public void shouldDoSomething() {
      service.someServiceMethod();
      verify(mockPersonDao).findAllPeople();
   }
}

Somewhere in the realms of test-context.xml we will configure the fact that PersonDao should be provided by a mock:

public class MyDependencyConfigTest extends MyDependencyConfig {
    @Bean
    public PersonDao personService() {
        return new MockPersonDao();
    }
}

The problem: Declarative programming

With a dependency injection container, the components in my code are loosely coupled. Indeed, this is the main selling point of dependency injection. But sometimes, loosely coupled code is the problem.

First, understanding which dependency is actually used (and why!) can require a lot of brain power: It will require you to consider all possible configurations when you look at a dependency and it will not work well with the normal code navigation functionality of your IDE. Second, the configuration will tend to deteriorate: When you no longer use a dependency, will you check whether you can remove it, or will you just leave it there to be safe?

Lastly, it can be hard to spot errors in the configuration:

I once had a service that needed had a @Resource TransactionManager and a @Resource DataSource. However, the system had a pair of each. The test code was correct, but in the production code, I had by accident configured the wrong TransactionManager. The effect was that the Service didn’t run in a transaction for the correct data source. That is: It didn’t really run in a transaction. The problem with this is that you only discover the problem if you scrutinize the contents of the database after the transaction was supposed to be rolled back.

Dependency injection in specific and declarative programming in general mean More Magic. More Magic is seldom a good thing, at least not when there are simple, time-tested strategies that work. Even if said strategies have fallen out of fashion.

This dependency injection madness must end!

Comments:

Johannes Brodwall - Nov 10, 2010

Nice! I’ve updated the code to use constructor injection.

In the test situation, I’ve seen both ways. Especially if the dependency is something like a DataSource or SessionFactory, people seem to prefer to get it from the container.

I agree with you that this is probably not smart.

Fredrik Kalseth - Nov 10, 2010

For one thing, you’re using property injection which is not the preferred way to do this. Secondly, you would “never” (maybe sometimes) use a container in your tests.

This is how your code would look if you did dependency injection the preffered way:

public class PersonService { private PersonDao _personDao; public PersonService(PersonDao personDao) {

_personDao = personDao;

}

public void someServiceMethod() { // Do something with personDao } }

then, in the test its very obvious that PersonService has a dependency on PersonDao - you need to give it one when you construct it. So you just do that:

@Test public void shouldDoSomething() { PersonService service = new PersonService(mockPersonDao); service.someServiceMethod(); verify(mockPersonDao).findAllPeople(); }

[Christin Gorman] - Nov 11, 2010

Well said! Like a jigsaw puzzle, you should be able to take your application apart and inspect each piece separately. But you should also be able to put the parts together and see the resulting picture. An application is more than the sum of all the individual classes. How the pieces actually fit together is as important as anything else and should be easily seen in the code.

[Eirik Maus] - Nov 14, 2010

I support your war on magic, but not on dependency injection. The problem is that you are using those evil annotations.

Annotation-based dependency-injection (essentially the same as dependency lookup), annotation-based transaction demarcation and probably most other automagic annotation-controlled features, removes the separation between the definition of the class and the use of the class. This turns an otherwise reusable and flexible class into a single-use-only class, where that single use is a production-like configuration (hopefully, but not necessarily, as the last codebase I inherited demonstrates…).

The fact that the annotations look so persuasively cool is just another token of their wickedness.

Dependencies between objects are probably the most important aspect of your application, and should be made visible. Good old spring xml-files have never been sexy, but at least they keep the configuration together in a single location that stands out from the rest of the codebase. Also, there are IDEs that understands spring xml as program code, providing code-completion, navigation from bean-tags to class definitions, refactoring of property names from the xml file and such, making them virtually painless compared to the alternative. But, alas, this is also a strategy that is out of fashion.

Johannes Brodwall - Nov 14, 2010

Man, you are TWISTED. Most of the so-called configuration is simply moving hardcoded Java-code into hardcoded XML-code. Centralizing all dependencies is as crazy as centralizing all places you use the “+” operator.

[Sjofel Onkel] - Nov 17, 2010

replacing well known and massively used Spring features with “home made” features can easily make the code base more difficult to understand.

Johannes Brodwall - Nov 17, 2010

Only true if Spring solved a problem you actually had. Which is the case in about 25% of the places I’ve seen it used.

Michael Semb Wever - May 13, 2011

I think what you are selling here is the use of Service Locators (or variation of it, through the use of sessionFactory) over Dependency Injection.

This is an important sell. We should be making a more conscientious decision between Service Locators and Dependency Injection in each project, rather than blindly following our own preference for everything at hand.

This article goes into this in depth: http://tech.finn.no/2011/05/13/dependency-injection-with-constructors/

[Bjørn Erik Haug] - Nov 26, 2010

But now your module containing the PersonService need a reference to the module with the HibernatePersonDao at compile time. A container will let you depend only on the abstraction.

Ara - Nov 10, 2010

Thats why I use managed beans

Johannes Brodwall - Nov 26, 2010

If you only have one implementation of PersonDao, is it really an abstraction?

[Kim] - Oct 1, 2012

Dependency injection frameworks really, really becomes fun when developers import dependency configuration files from “separate” and “isolated” modules. Oh they joys of projects where one module is overriding another modules dependencies by praying that the xml configuration files are loaded in the expected order. Even more so as these modules tend to evolve separately and at some point of time depend on different versions of the same dependency injection framework - and all their transient dependencies!

I find myself becoming more fond of hand coding most of the “plumbing” code. Doing this comes with quite a few positive side effects (in my experience), the biggest being that programs become easier to understand and reason about as creating deep and complicated dependency graphs by hand just becomes too impractical. Related code / features groups nicely together and can be used without xml files or knowledge about how the classes are wired together internally.

Johannes Brodwall - Oct 1, 2012

I like your approach, Kim. Christin Gorman says it really well: “I’m all for loose coupling, but do yourself a favor and make sure your solution is coupled with your problem.” http://kranglefant.tumblr.com/post/32472145738/losing-loose-coupling.

• English
• Java