2

Lately I find myself not caring too much defining dependencies for a class and passing them in the constructor, but I just always pass the DI container and keep it in a private attribute. This way my class won't have very well-defined dependencies, I get everything from the container when I need it.

Somehow I have bad feelings about this solution though (apart from the overhead caused by accessing the container), but I can't really think of too much disadvantages. Maybe having loose definition of dependencies can decrease portability of a class, or I get surprises when refactoring...?

What do you think about this?

gphilip
  • 1,114
  • 15
  • 33
  • 1
    Injecting the container into a class is just a form of the Service Locator anti-pattern and is (almost) as bad as calling a static Service Locator intance. – Steven May 01 '12 at 13:10
  • See also http://stackoverflow.com/questions/23931321/why-injecting-whole-service-container-to-another-service-is-almost-always-a-ba – Dan Blows Oct 01 '14 at 15:36

2 Answers2

11

Absolutely wrong. Don't put the DI container in the objects; they need not know or care that they're being injected. This doesn't square with "don't call us; we'll call you."

It's the other way 'round: the overall app knows about the DI engine, but then it gets the beans it needs from it.

I suppose you might argue that annotations change the relationship some, because now the beans do know something about the fact that they're wired together. But when configuration was externalized into XML, it was true that beans were ignorant of DI.

duffymo
  • 305,152
  • 44
  • 369
  • 561
3

This is an anti-pattern and I dare to bet that you are not writing tests yet for your code which is really bad.

When you start writing tests you will find that you are writing un-testable code, and you are:

  • Using the Service Locator anti-pattern

  • You are violating the Law of Demeter

  • You are not following the Single Responsibility Principle

  • You are hiding your object real dependencies (Not using the Dependency-Injection pattern)

  • You do not have a real separation between the two big piles of object in any application:

    • Business objects: Responsibility is business logic, domain abstractions
    • Wiring and building objects: Responsibility is to build object graphs

The day you want to write unit-tests for your components you will regret this decision and you will have either refactor your code or write integration tests instead of simple unit-tests (creating a test container and inject mocks to this container).

My advise is read this guide to write testable code: (this is the guide used by the guys working hard in Google)

http://misko.hevery.com/code-reviewers-guide/

if you prefer, start with these videos from Misko Hevery about the psychology of testing and clean code talks:

http://www.youtube.com/watch?v=wEhu57pih5w&feature=player_embedded

http://www.youtube.com/watch?v=RlfLCWKxHJ0&feature=player_embedded

http://www.youtube.com/watch?v=-FRm3VPhseI&feature=player_embedded

Jupaol
  • 21,107
  • 8
  • 68
  • 100
  • I think you are dramatizing the situation a bit here. I'm indeed writing unit tests to my classes and I don't see how defining fixtures in my DI container instead of passing them directly makes things any complicated. Nor I agree with your point about SRP, and only partly what you say about the law of demeter (it's a container). You imply here too many things. – gphilip Apr 28 '12 at 14:44