views:

358

answers:

7

How do we identify when to use dependency injection or singleton pattern. I have read in lot of websites where they say "Use Dependency injection over singleton pattern". But I am not sure if I totally agree with them. For my small or medium scale projects I definitely see the use of singleton pattern straightforward.

For example Logger. I could use Logger.GetInstance().Log(...) But, instead of this, why do I need to inject every class I create, with the logger's instance?.

+10  A: 

If you want to verify what gets logged in a test, you need dependency injection. Furthermore, a logger is rarely a singleton - generally you have a logger per each of your class.

Watch this presentation on Object-oriented design for testability and you'll see why singletons are bad.

The problem with singletons is that they represent a global state which is hard to predict, especially in tests.

Have in mind that an object can be de-facto singleton but still be obtained via dependency-injection, rather than via Singleton.getInstance().

I'm just listing some important points made by Misko Hevery in his presentation. After watching it you will gain full perspective on why it is better to have an object define what its dependencies are, but not define a way how to create them.

Bozho
special thanks for the link...:)
SysAdmin
+2  A: 

It's mostly, but not entirly about tests. Singltons were popular because it was easy to consume them, but there are a number of downsides to singletons.

  • Hard to test. Meaning how do I make sure the logger does right thing.
  • Hard to test with. Meaning if I'm testing code that uses the logger, but it's not the focus of my test, I still need to ensure my test env supports the logger
  • Sometimes you don't want a singlton, but more flexibilty

DI gives you the easy consumption of your dependent classes - just put it in the constructor args, and the system provides it for you - while giving you the testing and construction flexibility.

Scott Weinstein
Not really. It's about shared mutable state and static dependencies making things painful in the long run. Testing is just the obvious, and frequently the most painful, example.
kyoryu
+11  A: 

Singletons are like communism: they both sound great on paper, but explode with problems in practice.

The singleton pattern places a disproportionate emphasis on the ease of accessing objects. It completely eschews context by requiring that every consumer use an AppDomain-scoped object, leaving no options for varying implementations. It embeds infrastructure knowledge in your classes (the call to GetInstance()) while adding exactly zero expressive power. It actually lessens your expressive power, because you can't change the implementation used by one class without changing it for all of them. You simply can't add one-off pieces of functionality.

Also, when class Foo depends on Logger.GetInstance(), Foo is effectively hiding its dependencies from consumers. This means that you can't fully understand Foo or use it with confidence unless you read its source and uncover the fact that it depends on Logger. If you don't have the source, that limits how well you can understand and effectively use the code on which you depend.

The singleton pattern, as implemented with static properties/methods, is little more than a hack around implementing an infrastructure. It limits you in myriad ways while offering no discernible benefit over the alternatives. You can use it however you'd like, but as there are viable alternatives which promote better design, it should never be a recommended practice.

Bryan Watts
I love your analogy %)
Krzysztof Koźmic
+1: apart from the distracting analogy (not necessarily wrong, but a bit silly), this may be the best summary of the problems with singletons I've read.
Jeff Sternal
It was intended as tongue-in-cheek. There's nothing wrong with a little shock value :-)
Bryan Watts
+2  A: 

Others have explained very well the problem with singletons in general. I would just like to add a note about the specific case of Logger. I agree with you that it is usually not a problem to access a Logger (or the root logger, to be precise) as a singleton, via a static getInstance() or getRootLogger() method. (unless if you want to see what gets logged by the class you are testing - but in my experience I can hardly recall such cases where this was necessary. Then again, for others this might be a more pressing concern).

IMO usually a singleton logger is not a worry, since it does not contain any state relevant to the class you are testing. That is, the logger's state (and its possible changes) have no effect whatsoever on the state of the tested class. So it does not make your unit tests any more difficult.

The alternative would be to inject the logger via the constructor, to (almost) every single class in your app. For consistency of interfaces, it should be injected even if the class in question does not log anything at present - the alternative would be that when you discover at some point that now you need to log something from this class, you need a logger, thus you need to add a constructor parameter for DI, breaking all client code. I dislike both of these options, and I feel that using DI for logging would be just complicating my life in order to comply with a theoretical rule, without any concrete benefit.

So my bottom line is: a class which is used (almost) universally, but does not contain state relevant to your app, can safely be implemented as Singleton.

Péter Török
Even then, a singleton can be a pain. If you realize your logger needs some additional parameter for some cases, you either get to make a new method (and let the logger get uglier), or break all consumers of the logger, even if they don't care.
kyoryu
@kyoryu, I am talking about the "usual" case, which IMHO implies using a (de facto) standard logging framework. (Which typically are configurable via property / XML files btw.) Of course there are exceptions - as always. If I know my app is exceptional in this respect, I won't use a Singleton indeed. But overengineering saying "this might be useful sometime" is almost always a wasted effort.
Péter Török
If you're already using DI, it's not much extra engineering. (BTW, I don't disagree with you, I'm the upvote). A lot of loggers require some "category" information or something of the sort, and adding additional parameters can cause pain. Hiding it behind an interface can help keep the consuming code clean, and can ease switching to a different logging framework.
kyoryu
@kyoryu, sorry for the wrong assumption. I saw a downvote and a comment, so I connected the dots - the wrong way, as it turns out :-( I have never experienced the need to switch to a different logging framework, but then again, I understand it may be a legit concern in some projects.
Péter Török
Correct me if I am wrong, but the singleton would be for the LogFactory, not the logger. Plus the LogFactory is likely to be the apache commons or Slf4j logging facade. So switching logging implementations is painless anyway. Isn't the real pain with using DI to inject a LogFactory the fact that you have to go to the applicationContext now to make every instance in your app?
HDave
@HDave, exactly.
Péter Török
+1  A: 

About the only time you should ever use a Singleton instead of Dependency Injection is if the Singleton represents an immutable value, such as List.Empty or the like (assuming immutable lists).

The gut check for a Singleton should be "would I be okay if this was a global variable instead of a Singleton?" If not, you're using the Singleton pattern to paper over a global variable, and should consider a different approach.

kyoryu
A: 

Just checked out the Monostate article - it's a nifty alternative to Singleton, but it has some wierd properties:

class Mono{
    public static $db;
    public function setDb($db){
       self::$db = $db;
    }

}

class Mapper extends Mono{
    //mapping procedure
    return $Entity;

    public function save($Entity);//requires database connection to be set
}

class Entity{
public function save(){
    $Mapper = new Mapper();
    $Mapper->save($this);//has same static reference to database class     
}

$Mapper = new Mapper();
$Mapper->setDb($db);

$User = $Mapper->find(1);
$User->save();

Isn't this kind of scary - because the Mapper is really dependent on the database connection to perform save() - but if another mapper has been created prior - it can skip this step in acquiring it's dependencies. While neat, it's also kind of messy isn't it?

sunwukung