by Miško Hevery
After reading the article on Singletons (the design anti-pattern) and how they are really global variables and dependency injection suggestion to simply pass in the reference to the singleton in a constructor (instead of looking them up in global state), many people incorrectly concluded that now they will have to pass the singleton all over the place. Let me demonstrate the myth with the following example.
Let’s say that you have a LoginPage which uses the UserRepository for authentication. The UserRepository in turn uses Database Singleton to get a hold of the global reference to the database connection, like this:
class UserRepository {
private static final BY_USERNAME_SQL = "Select ...";
User loadUser(String user) {
Database db = Database.getInstance();
return db.query(BY_USERNAME_SQL, user);
}
}
class LoginPage {
UserRepository repo = new UserRepository();
login(String user, String pwd) {
User user = repo.loadUser(user);
if (user == null || user.checkPassword(pwd)) {
throw new IllegalLoginException();
}
}
}
The first thought is that if you follow the advice of dependency injection you will have to pass in the Database into the LoginPage just so you can pass it to the UserRepository. The argument goes that this kind of coding will make the code hard to maintain, and understand. Let’s see what it would look like after we get rid of the global variable Singleton Database look up.
First, lets have a look at the UserRepository.
class UserRepository {
private static final BY_USERNAME_SQL = "Select ...";
private final Database db;
UserRepository(Database db) {
this.db = db;
}
User loadUser(String user) {
return db.query(BY_USERNAME_SQL, user);
}
}
Notice how the removal of Singleton global look up has cleaned up the code. This code is now easy to test since in a test we can instantiate a new UserRepository and pass in a fake database connection into the constructor. This improves testability. Before, we had no way to intercept the calls to the Database and hence could never test against a Database fake. Not only did we have no way of intercepting the calls to Database, we did not even know by looking at the API that Database is involved. (see Singletons are Pathological Lairs) I hope everyone would agree that this change of explicitly passing in a Database reference greatly improves the code.
Now lets look what happens to the LoginPage…
class LoginPage {
UserRepository repo;
LoginPage(Database db) {
repo = new UserRepository(db);
}
login(String user, String pwd) {
User user = repo.loadUser(user);
if (user == null || user.checkPassword(pwd)) {
throw new IllegalLoginException();
}
}
}
Since UserRepository can no longer do a global look-up to get a hold of the Database it musk ask for it in the constructor. Since LoginPage is doing the construction it now needs to ask for the Databse so that it can pass it to the constructor of the UserRepository. The myth we are describing here says that this makes code hard to understand and maintain. Guess what?! The myth is correct! The code as it stands now is hard to maintain and understand. Does that mean that dependency injection is wrong? NO! it means that you only did half of the work! In how to think about the new operator we go into the details why it is important to separate your business logic from the new operators. Notice how the LoginPage violates this. It calls a new on User Repository. The issue here is that LoginPage is breaking a the Law of Demeter. LoginPage is asking for the Database even though it itself has no need for the Database (This greatly hinders testability as explained here). You can tell since LoginPage does not invoke any method on the Database. This code, like the myth suggest, is bad! So how do we fix that?
We fix it by doing more Dependency Injection.
class LoginPage {
UserRepository repo;
LoginPage(UserRepository repo) {
this.repo = repo;
}
login(String user, String pwd) {
User user = repo.loadUser(user);
if (user == null || user.checkPassword(pwd)) {
throw new IllegalLoginException();
}
}
}
LoginPage needs UserRepository. So instead of trying to construct the UserRepository itself, it should simply ask for the UserRepository in the constructor. The fact that UserRepository needs a reference to Database is not a concern of the LoginPage. Neither is it a concern of LoginPage how to construct a UserRepository. Notice how this LoginPage is now cleaner and easier to test. To test we can simply instantiate a LoginPage and pass in a fake UserRepository with which we can emulate what happens on successful login as well as on unsuccessful login and or exceptions. It also nicely takes care of the concern of this myth. Notice that every object simply knows about the objects it directly interacts with. There is no passing of objects reference just to get them into the right location where they are needed. If you get yourself into this myth then all it means is that you have not fully applied dependency injection.
So here are the two rules of Dependency Injection:
- Always ask for a reference! (don’t create, or look-up a reference in global space aka Singleton design anti-pattern)
- If you are asking for something which you are not directly using, than you are violating a Law of Demeter. Which really means that you are either trying to create the object yourself or the parameter should be passed in through the constructor instead of through a method call. (We can go more into this in another blog post)
So where have all the new operators gone you ask? Well we have already answered that question here. And with that I hope we have put the myth to rest!
BTW, for those of you which are wondering why this is a common misconception the reason is that people incorrectly assume that the constructor dependency graph and the call graph are inherently identical (see this post). If you construct your objects in-line (as most developers do, see thinking about the new operator) then yes the two graphs are very similar. However, if you separate the object graph instantiation from the its execution, than the two graphs are independent. This independence is what allows us to inject the dependencies directly where they are needed without passing the reference through the intermediary collaborators.
31 responses so far ↓
I think where people sometimes get confused here, is when they start to apply questions of scope to this problem. So far, you’re putting forth an example that involves singletons, except that UserRepository is created once per LoginPage. Because we don’t know the true requirements (it’s example code) we have to make assumptions, but let’s assume that you can’t turn UserRepository into a small-s singleton (one instance per application). How do you get the right number of creations?
What you’ve done, by moving “new” into the container, actually helps solve this problem as well. Because, depending on the capabilities of the container you’ve chosen, you can have the container determine the scope of the item.
Looking at Spring for a second (I don” know Guice well enough to use a guice example), you could register the UserRepository as a singleton scope, or you could register it as a prototype scope, which means a new one is created every time it is asked for. Or, because this is a web-application (I presume), you could create one per-session, which seems more appropriate.
In this approach, by registering a singleton-scoped Database, but a session-scoped UserRepository, the container, when it creates LoginPage, provides the same UserRepository instance for the life of the session, and each UserRepository is created with a reference provided to a single instance of Database. And all that wiring instruction is defined in the registration of the components with the container.
So your examle code still works, and you have the right scopes. You don’t lose the scoping by using a container – you just use containers that support scope.
I think where people sometimes get confused here, is when they start to apply questions of scope to this problem. So far, you’re putting forth an example that involves singletons, except that UserRepository is created once per LoginPage. Because we don’t know the true requirements (it’s example code) we have to make assumptions, but let’s assume that you can’t turn UserRepository into a small-s singleton (one instance per application). How do you get the right number of creations?
What you’ve done, by moving “new” into the container, actually helps solve this problem as well. Because, depending on the capabilities of the container you’ve chosen, you can have the container determine the scope of the item.
cont’d…
Looking at Spring for a second (I don” know Guice well enough to use a guice example), you could register the UserRepository as a singleton scope, or you could register it as a prototype scope, which means a new one is created every time it is asked for. Or, because this is a web-application (I presume), you could create one per-session, which seems more appropriate.
In this approach, by registering a singleton-scoped Database, but a session-scoped UserRepository, the container, when it creates LoginPage, provides the same UserRepository instance for the life of the session, and each UserRepository is created with a reference provided to a single instance of Database. And all that wiring instruction is defined in the registration of the components with the container.
So your examle code still works, and you have the right scopes. You don’t lose the scoping by using a container – you just use containers that support scope.
Or more to the point, the components can be tested without reference to scope, and they can be used in other contexts or scopes if it’s appropriate. Because you don’t encode scope assumptions into the components themselves, your code is inherently more re-usable on that dimension as well.
Fancy words!
But this is a solid implementation pattern.
I’m always digging through code which spends most of it’s time looking all over for things it shouldn’t really know or care about before it finally executes one or two lines of business.
If you try to write a unit test you end up having to refactor this fassion anyway.
I wasn’t arguing with dependency-injection, I was pointing out a case that some people fear, and showing that it is still valid.
And I agree with your last statement, about having to re-factor into this sort of structure if you write unit tests… the problem is getting people to write unit tests.
Pass Around Ginormous Context Objects | Miško Hevery // Oct 27, 2008 at 2:29 pm
[...] about the parameters, and its caller will have to know and so on. We talked about this in ‘Dependency Injection Myth: Reference Passing‘. However, we are making one more mistake. We are mixing object creation (and look up) with [...]
Michael Tsai - Blog - Dependency Injection Myth: Reference Passing // Nov 4, 2008 at 9:28 am
[...] Miško Hevery (blog via Dave Dribin): [...]
Hi Miško,
Thanks for a great blog.
>>Guess what?! The myth is correct! The code as it stands now is hard to maintain and understand
This is clearly demonstrable by looking at the number of dependencies involved.
Initially, with all dependencies hardcoded we have (–> = “depends on”):
UserRepository –> Database
LoginPage –> UserRepository
After refactoring halfway:
UserRepository –> Database
LoginPage –> UserRepository AND Database
This situation is worse as the number of dependencies has increased for LoginPage.
After completing DI we are back to:
UserRepository –> Database
LoginPage –> UserRepository
but the dependencies are now parameterised rather than hardcoded.
Steve
Hi Miško,
you could also use a class factory in this case:
Database db = DatabaseFactory.Create();
So you can create your mock version without modifying anything.
@Thiago,
I disagree. Since DatabaseFactory.Create() is a static method and takes no arguments it must always return the same thing. Which is equivalent as saying new Database(). If you are thinking that DatabaseFactory.Create(); will return different things, than you must have global state whith DatabaseFactory.Create(); interacts with. And global state is evil:
http://misko.hevery.com/2008/11/21/clean-code-talks-global-state-and-singletons/
http://misko.hevery.com/2008/08/25/root-cause-of-singletons/
Hello Misko,
What about the drawback of increasing memory consumption? Each object using a global variable gets now an additional member variable instead.
Best regards,
Tilo
I see your point.
I was ingenuous in thought about changing DatabaseFactory.Create() using a Mock to Database for testing, when needed. In this case, I will really end up using a global state to change its return. Thanks for remember me.
@TiloOne additional reference variable is trivial compared to the size of your application’s memory footprint. There’s really only a very small memory usage difference between a globally accessible instance and a single instance shared amongst several other objects. Also, you shouldn’t worry about minor memory use increase until you really start profiling your application, and then worry about it only if you’re building a time-critical or low-resource environment application.
While I agree with the sentiment here, I think there’s a bit of sleight-of-hand going on with these examples. With the Singleton example, it’s obvious how the UserRepository gets its db connection – it “fetches” it using a call to the Singleton, which probably makes a connection to the db if it doesn’t already exist. In the DI example, it’s not clear how the UserRepository gets a working db connection – basically it is just saying, “I’m not doing anything until you give me a db connection”. I suppose the assumption here is that there is some sort of DI framework or other implementation that actually injects an existing db connection. That may be, but it’s misleading to say, “Look how much cleaner the second example is” because you have stripped out the part of the logic that actually creates and injects a db connection into the object. How do we know that you haven’t just shifted the problem to some other layer of your application, eg. the DI framework?
@Darren,
You have shifted the problem to the factory. Bu that is a good thing for many reasons. (1) Separation of concerns. (2) Separating of the initialization logic from the business logic and (3) guaranteed order of initialization.
So it is now obvious that the application factory (or the container) holds the small-s-singleton: an instance of the Database object. What about construction of objects that have a small lifetime? For instance, a ArticleRepository could create Tag objects to send when a Article is modified; my approach is to make ArticleRepository to ask for a TagFactory in the constructor. This is an example of creation of non-newables (Song example) in the application logic, that is simply delegation to the TagFactory. It could be the same with any other object that is not created for the global application lifetime but only for a small one, and that has in the constructor some dependency to a general singleton like Database.So the TagFactory needs to hold a reference to pass to the Tag, right?
@Giorgio,
I am having a hard time follewing your example without an example code, but I think you are on the right track and what you are saying is correct.
It looks clean, but isn’t this requiring a god class that needs to create everything? If you want to create a LoginPage, you have to create a UserRepository and a Database. This becomes a pain when you move outside an example to deeper and wider dependencies.
@ Ryan,
why do you need a God class? Everyone just knows what they need to know. Factory knows about everything which it is responsible for instantiating. Where does God class come in?
@Misko:I found your article Where have all the singletons gone?… Translating in that example, I was worried that RequestFactory has to know about everything the objects it creates need. After coding I think it makes sense, even if RequestFactory creates third-level factories to pass in the constructor of objects that absolutely need to create their dependency dinamically. My coding was on a Mapper for database tables that needs to create a table Introspector for each record and subrecord it has to save. I resolved with a IntrospectorFactory.
Misko, I agree best to request dependencies in the constructor for most case but how do you suggest DI for something as ubiquitous as a logging object? Since I think you advise against factories and I don’t want to cascade the logging interface request into every constructor that might log, I’m at a loss on how to do this efficiently and cleanly. I’m thinking about using a proxy factory that a delegate must be set into by the production or the testing application. If you recommend using a container, how is that different from a factory and how do you normally access that container from objects which may have been instantiated by a production vs. a testing container? Again the only way I can think of is to have a container proxy which has a delegate set into it by the actual container. Maybe container and factory are really the same thing? Thanks, Dave
@Dave,
Logging is a weird case, because the information flows only one way, from your application to the logger. More importantly your application does not (should not behave any differently) if the logging is enabled or disabled. For this reason even thought logging is global state it will not hurt you, so I say just do Logger.getLog(…) right where you need it. However, if in your test you need to assert that a particular message did get written to a log, than you have no choice but to inject the logger through the constructor. GUICE will do this for you automatically.
Test Driven Development // Nov 17, 2009 at 9:34 am
[...] way I approach programming. When to use Dependency Injection, Guide to Writing Testable Code and Dependency Injection Myth: Reference Passing are some of the blog posts that I would definitely recommend [...]
Singleton « Blog HM // Jul 25, 2010 at 6:56 am
[...] Nếu tính chất “Singleton” không phải là bản chất của class, mà là do class dùng nó yêu cầu thế, thì tính duy nhất nên được hiện thực theo một cách độc lập, như Factory Method hay Dependency Injection. [...]
Hi Misko, thanks for great article.
I really like the concept of DI, i just want to ask about one thing.
It seems to me, passing Database to UserRepository constructor is violation of encapsulation and principle of minimal knowledge.
Why should i care (know) about where User object is come from and what is hidden underneath.
If we have House object that needs Windows and Doors, it looks clean, because Windows and Doors are in one layer with House.
But UserRepository and Database are from different layers. The same is for LoginPage and UserRepository.
What do you think about this?
Thanks.
@Yuriy,
I think this is already answered here; http://misko.hevery.com/2008/10/21/dependency-injection-myth-reference-passing/
Using dependency injection in a PHP web application | Gravity Layouts // Oct 26, 2011 at 8:36 pm
[...] to Miško Hevery, I should not be passing references to dependencies through the different layers of my application. [...]
Using dependency injection in a PHP web application | SeekPHP.com // Oct 26, 2011 at 9:13 pm
[...] to Miško Hevery, I should not be passing references to dependencies through the different layers of my application. [...]
Are singletons pathalogical liars? | Davelog // Dec 31, 2011 at 4:57 pm
[...] The response is apparently answered in the post Dependency Injection Myth: Reference Passing [...]
Misko,
I suspect I am not truly seeing the light w.r.t dependency injection, but maybe you can help me with an example.
I have a system where I have a bunch of different plugins all implementing the same interface, such as a database connectors, or version control connectors, that kind of thing.
I then have a bunch of services that those plugins might want to access, all defined in terms of interfaces. For example a user interactor for authentication, character encoding translation services, configuration services and so on. There will be numerous such services. Not all plugins use all services.
I can see how to connect this up using a Service Locator type pattern, with the plugins accessing just the services they need through a supplied Service Locator, but I don’t like the hidden dependencies this creates. However I don’t see how else I can connect things up.
Note that this is C++, and there is no magic “dependency injection container” involved, just plain old C++ code.
I suspect I need a layered approach where I write the bulk of the plugin with explicit dependencies, but the final “glue” has to take what amounts to a Service Locator, extracts just the services that the plugin actually needs, and passes them to the main part of the plugin. That “glue” is probably what a dependency injection container would do automatically if I were using one.
Leave a Comment