I'm using Dependency Injection: which types should I bind as singletons?

Question

There are a lot of questions out there about whether singletons are "bad," and what patterns to use instead. They're generally focused on the singleton design pattern, which involves retrieving the singleton instance from a static method on the class. This is not one of those questions.

Ever since I really "discovered" dependency injection several months back, I've been driving its adoption in our team, removing static and singleton patterns from our code over time, and using constructor-based injection wherever possible. We've adopted conventions so that we don't have to keep adding explicit bindings to our DI modules. We even use the DI framework to provide logger instances, so that we can automatically tell each logger which class it's in without additional code. Now that I have a single place where I can control how various types are bound, it's really easy to determine what the life cycle of a particular category of classes (utility classes, repositories, etc.) should be.

My initial thinking was that there would probably be some advantage to binding classes as singletons if I expected them to be used fairly often. It just means there's a lot less newing going on, especially when the object you're creating ends up having a big dependency tree. Pretty much the only non-static fields on any of these classes are those values getting injected into the constructor, so there's relatively little memory overhead to keeping an instance around when it's not being used.

Then I read "Singleton I love you, but you're bringing me down" on www.codingwithoutcomments.com, and it made me wonder whether I had the right idea. After all, Ninject compiles object-creation functions by default, so there's very little reflection overhead involved in creating additional instances of these objects. Because we're keeping business logic out of the constructor, creating new instances is a really lightweight operation. And the objects don't have a bunch of non-static fields, so there isn't a lot of memory overhead involved in creating new instances, either.

So at this point, I'm beginning to think that it probably won't matter much either way. Are there additional considerations I haven't taken into account? Has anyone actually experienced a significant improvement in performance by changing the life cycles of certain types of objects? Is following the DI pattern the only real important thing, or are there other reasons that using a single instance of an object can be inherently "bad?"

Answer 1

It shouldn't matter much for memory usage, but you'll get increased modularity, easier testability with mock objects, and the aesthetic ugliness of having to write out dependencies explicity will encourage programmers to make each class more orthogonal and independent. It's easier to gloss over dependencies when your class is calling globals, which a singleton basically is, and it can make your life harder later.

Answer 2

I am not sure if memory management is the big issue unless you are creating A LOT of objects. I found using singleton from my IoC container helpful when dealing with something like caching, where creating the caching object and making a connection to the caching servers is expensive during creation so we create the caching object once and reuse it.

A single instance can be harmful however cause it can create a bottleneck in performance as you have a single instance serving multiple requests in the case of services.

Answer 3

As noted in Gary's answer, the whole point of DI is testability (you can easily mock out all references to a class since they are all listed in the constructor), not runtime performance.

You want to do TDD (test-drive development), right?

Answer 4

I am using singleton instances to cache expensive to create objects (like nhibernate session factory) or for objects I want to share across the application.

If in uncertainty I would rather have the object be recreated every time it is used and mark it as singleton or "per thread"/"per request"/"per whatever" only if you really need to.

Scattering singletons all over the place to potentially save some newing up is premature optimization and can lead to really nasty bugs because you can't be sure if the object is new or shared across multiple objects/instances.

Even if the object has no state at the moment and seems save to share, someone can refactor later and add state to it which then may be unintentionally shared between different objects/instances.