Dependency Injection: a discussion of the pros and cons

[s_molinari]

This again has turned into another useless discussion and why I said Tony doesn’t deserve even the thread.

This reminds me of a great Mark Twain quote.

“Never argue with a fool, onlookers may not be able to tell the difference.”

Scott

[Hall_of_Famer]

This thread is getting quite interesting, so here are my two cents:

@tony_marston404

I keep hearing the phrase “singletons are widely acknowledged to be a bad idea” but without any explanation as to precisely WHY they are a bad idea. Unless you can provide me with a code sample that identifies the problem I shall continue to treat this notion as an unsubstantiated opinion.

Nope, people have been giving you explanations after explanations on precisely why Singleton is bad. But you dont remember them, since you refuse to listen, and instead consider their comments as personal insults. It does not matter whether you hear them or not, they do objectively exist anyway, which makes your point invalid. Singleton is an antipattern and there’s no room for even arguing on this aspect. I think you should understand better if you start to take constructive criticism nicely and do not hold this ‘I have much longer programming experience than you’ attitude.

@tony_marston404

Dependency Injection has nothing to do with encapsulation, and encapsulation does not mean “information hiding”, it actually means “implementation hiding”.

Dependency Injection has A LOT to do with encapsulation, because it offers a way to achieve better information hiding. The alternative to Dependency Injection, your singleton(which are effectively glorified global variables), it totally breaks encapsulation. I cant say DI is a perfect solution, but at least it does not break encapsulation like Singleton does.

@tony_marston404

I would probably not be wrong in saying that the levels of reusability which I achieve with my framework are far superior to anything which you could achieve.

First of all, can you define what is ‘superiority’? And next, what is this so-called level of reusability in your framework?

Anyway I agree with @WebMachine that this thread has gone in an unfavorable direction and that it has strayed from the original intention by the OP @fretburner. But anyway, I dont see it really going anywhere, until Tony realizes that the counter-arguments and constructive criticisms are not meant for personal insulting, but for valid discussion. Or maybe, Tony actually does this on purpose since if he acts like he ‘is angered or hurt by insulting comments’, he will have a better chance to digress(like he did at post #46) and make the rest of us lose focus on what we are discussing. umm smart strategy if you ask me.

[fretburner]

So, several people have given good examples of the benefits of DI but what about the flip-side of the discussion, are there any situations where you wouldn’t use it? Perhaps some of you guys could comment on that?

[TomB]

@fretburner

So, several people have given good examples of the benefits of DI but what about the flip-side of the discussion, are there any situations where you wouldn’t use it? Perhaps some of you guys could comment on that?

That’s a good question.

There are some cases where using DI presents a problem (more of a hurdle). For example. where you need to supply constructor arguments after some logic has run e.g.

class Dependency {
	public function __construct($foo) {

	}
}

class A {
	
	public function doSomething() {
		$x = uniqid();
		$dependency = new Dependency($x);
	}
}

Here, you cannot pass Dependency into A as a constructor argument because $x is worked out on the fly in A::doSomething().

There are two problems with this approach, however,

1. What if Dependency has a Dependency?

classs D {}

class Dependency {
	public function __construct(D $d, $foo) {

	}
}

Now the Dependency class has a dependency on D. There are two ways to solve this

A) Pass the dependency D into A

class A {
	private $d;

	public function __construct(D $d) {
		$this->d = $d;
	}
	
	public function doSomething() {
		$x = uniqid();
		$dependency = new Dependency($this->d, $x);
	}
}

This works and seems like an adequate solution but it’s inflexible. Imagine have another class, B which also constructs Dependency

class A {
	private $d;

	public function __construct(D $d) {
		$this->d = $d;
	}
	
	public function bDoesSomething() {
		$x = md5(microtime());
		$dependency = new Dependency($this->d, $x);
	}
}

And what if that is also happening in a dozen other classes? I need to pass a D object into every object that is going to construct Dependency. At first glance this doesn’t seem to be an issue. However, there’s some repeated code here which is almost always symptom of a problem and that problem is: If the implementation of Dependency changes to require a second dependency Z You have to locate and alter every single class that is instantiating Dependency to also ask for Z to pass in when it creates Dependency:

class Dependency {
	public function __construct(D $d, Z $z, $foo) {

	}
}

class A {
	private $d;
	private $z;

	public function __construct(D $d, Z $z) {
		$this->d = $d;
		$this->z = $z;
	}
	
	public function doSomething() {
		$x = uniqid();
		$dependency = new Dependency($this->d, $this->z, $x);
	}
}

Oh and of course, whatever is instantiating A, B and the dozen other classes that contain the code new Dependency() has to be updated as well!

What I’ve seen happen in this scenario in real life is that Dependency becomes essentially immutable. Nobody on the team dares alter Dependency::__construct() because they don’t want the hassle of fixing all the code that would break. (If you’re using TDD it’s a lot easier, but where I was working at the time we only had test cases for about 10% of the codebase because the code was mostly from over a decade ago) and you end up with copy/paste or inheritance based solutions which both cause their own set of problems.

B) The real solution is to make sure you only ever call new Dependency() in one place. By doing that, you ensure that making changes to Dependency::__construct doesn’t cause you any issues.

class DependencyFactory {
	public function __construct(D $d, Z $z) {
		$this->d = $d;
		$this->d = $z;	
	}

	public function create($x) {
		return new Dependency($this->d, $this->z, $x);
	}
}



class A {
	private $factory;

	public function __construct(DependencyFactory $factory) {
		$this->factory = $factory;
	}
	
	public function doSomething() {
		$x = uniqid();
		$dependency = $this->factory->create($x);
	}
}

class B {
	private $factory;

	public function __construct(DependencyFactory $factory) {
		$this->factory = $factory;
	}
	
	public function bDoesSomething() {
		$x = md5(microtime());
		$dependency = $this->factory->create($x);
	}
}

Now the implementation of Dependency::__construct() can be changed without having to alter the A, B and ancy other classes that need to construct instances of it.

2. The second problem with the original approach is that “new Dependency” is hardcoded. There’s no way to instantiate an object of any type other than the Dependency class. Polymorphism allows us to use a subclass in place of the original class, e.g. I may want to instantiate a subclasss of Dependency that is doing some logging or working in a slightly different way. By hardcoding new Dependency() this is impossible, if I want one A instance that has the logging Dependency and one that doesn’t I can’t (Other than an ugly if statement and constructor argument which allows switching). By using the factory, I can pass an instance of DependencySubClassFactory rather than DependencyFactory into A:__construct() and I have the flexibility to do that!

In short: Sometimes using DI requires some forethought and upfront design, but the benefits of doing so are worthwhile.

[s_molinari]

Again, great post Tom!

And for my sake, before Tony says, “See, you say DI has benefits, but don’t say what they are and thus don’t prove at all why DI should even be used, which proves my point, it shouldn’t be used”, can you please note, one more time, those benefits? I know them, many others know them, Tony ignores them, but please, note them again.

Scott

[tony_marston404]

@cpradio

DI has been a huge benefit to me and the way that I code as I can write better tests. Better tests means I can churn out changes quicker, as I simply write a new test for the upcoming change, write the code, and then verify all tests still pass.

If you are happy with DI then go for it. I personally do not see the benefit of DI in the kind of applications which I write, and I resent the attitude of the “paradigm police” that just because THEY like DI that I should too, and if I don’t follow their “advice” (which I interpret as diktats) then I must be some sort of heretical deviant.

@cpradio

Since going this route, the number of bugs produced from my changes have been zero. We’ve found issues with requirements or maybe a requirement was written incorrectly, but from what the requirements had, their test cases/scenarios, and the final result of development, zero bugs.

I don’t need DI to produce bug free software. I don’t need DI to add enhancements to my software. My point is that the effort of refactoring my entire codebase, which contains over 2,000 units of work, would be absolutely enormous. For what benefit? To make it easier to do something which I never have to do.

@cpradio

I unfortunately can’t say the same for some of the legacy systems that our devs do not write tests for, are riddled with tightly coupled dependencies, and although they “function”, the amount of time spent on fixing bugs, adding/updating functionality there is cost heavy and typically results in a new bug or two or dozen that most of the time gets caught in QA, but have edge cases that make it to Production as well.

Even DI cannot help with badly structured or badly written code. It is not a “magic bullet” that can automatically solve all your problems. All it does is move the problem from one area to another, and in the process it introduces more complexity, more levels of indirection, more code to execute, more code to read and understand, and the potential for more bugs.

@cpradio

That may be my experience and your experience may be entirely different, but if I were to start a project again from the ground up, I’d still follow a Test Driven Development path and ensure that my components can be fully tested and that would mean the usage of DI as a must.

Using DI is an option, not a requirement. TDD is an option, not a requirement. I have been happily designing and developing software for several decades without using those options, and I do not believe that using those options would make either the design process or the development process any quicker or better. That is my opinion, and I’m sticking to it.

[TomB]

@tony_marston404

If you are happy with DI then go for it. I personally do not see the benefit of DI in the kind of applications which I write, and I resent the attitude of the “paradigm police” that just because THEY like DI that I should too, and if I don’t follow their “advice” (which I interpret as diktats) then I must be some sort of heretical deviant.

Once again, this is refraining the issue. You cannot understand the benefits of DI yet you’re coming here arguing that it doesn’t have any benefits. They are two different issues. Your lack of understanding is not an argument in itself and writing articles and arguing that “DI is evil” yet providing no sources and code examples that uses known bad practices is detrimental to others. FYI I’m not participating in this topic to enlighten you because “That is my opinion, and I’m sticking to it.” is essentially your entire argument. I’m here for onlookers who may be beginning their journey into best-practices and this kind of topic leads to confusion. You’re trying to argue against the use of DI because you don’t understand encapsulation rather than arguing against DI because there is a better alternative.

There are two different issues here and you’re conflating them in order to prolong what is essentially an argument that never needed to exist. This is essentially like having a debate with a Christian Scientist. All the evidence in the world shows modern medicine works and prayer alone does not but they won’t budge from their original position no matter how much you prove them wrong yet to an outsider who knows nothing or very little about medicine it looks like there is some debate among the medical community when there actually isn’t. You can’t understand the benefits of modern medicine and insist that prayer has worked for you your whole life so modern medicine is rubbish.

[TomB]

@tony_marston404

and in the process it introduces more complexity, more levels of indirection, more code to execute, more code to read and understand, and the potential for more bugs.

This is a flat out lie.

This:

class Foo {

}

class Bar { 
	public function __construct(Foo $foo) {
		$this->foo = $foo;
	}
}

new Bar(new Foo);

is fewer lines of code, less complex and fewer method calls executed than

class Foo {
	private static $instance;

	public static function getInstance() {
		if (empty(self::$instance)) {
			 self::$instance = new Foo;
		}

		return self::$instance;
	}
}

class Bar { 
	public function __construct() {
		$this->foo = Foo::getInstance();
	}
}

new Bar();

[tony_marston404]

@TomB

This example is irrelevant and does not address my point at all. We’re talking about the pros/cons of DI in general. I don’t want specific implementation details.

I am only concerned with the benefits of DI in the kind of applications which I actually write, not the applications which I don’t write. I do not write software that builds ships and/or castles, I write software that enables users to get data into and out of a relational database. My framework was specifically designed to deal with this type of application and no other. I have very few dependencies, and I have absolutely no problem in dealing with those dependencies without the use of a DI mechanism.

@TomB

However, I’ll bite. How do do you assign validation rules to a table? What if you have a rule that is relevant to mutliple tables (e.g. names cannot contain numbers)?

I have a separate class for each table in the database, and this is generated from my data dictionary in the form of a class file and a data structure file. The class file looks like this:

<?php
require_once 'std.table.class.inc';
class #tablename# extends Default_Table
{
        function #tablename# ()
        {
        // save directory name of current script
        $this->dirname     = dirname(__file__);
        
        $this->dbname      = '#dbname#';
        $this->tablename   = '#tablename#';
        
        // call this method to get original field specifications
        // (note that they may be modified at runtime)
        $this->fieldspec = $this->getFieldSpec_original();
        
    } // #tablename#
    
} // end class
?>

This is described in more detail at http://www.tonymarston.net/php-mysql/data-dictionary.html#table.class.inc

The getFieldSpec_original() method loads the contents of the screen structure file which contains a list of the all the fields which exist in that table along with their specifications. An example can be found at http://www.tonymarston.net/php-mysql/data-dictionary.html#table.dict.inc

Because I now have an array of field names with their specifications, when an array of field names and field values is passed in (usually from the $_POST array) it is very easy to have a standard method of ensuring every field’s value matches its specifications. This standard method is built into the framework and is applied automatically without the developer having to write a single line of code.

There is no method of applying the same rule to multiple tables as each table must have its own set of rules. Any non-standard rules would have to be coded into one of the customisable methods as described in http://www.tonymarston.net/php-mysql/infrastructure-faq.html#faq13

@TomB

I keep hearing the phrase “singletons are widely acknowledged to be a bad idea” but without any explanation as to precisely WHY they are a bad idea. Unless you can provide me with a code sample that identifies the problem I shall continue to treat this notion as an unsubstantiated opinion.

I gave you this list before, but here it is again, not that you’ll read it.

I have read through that list, but I find the reasoning to be hypothetical and irrelevant to my circumstances. I use singletons simply because I may need to use a particular object in more than one place during the execution of a script, and rather than creating a brand new instance each time I prefer to reuse the one which I created earlier. Those articles talk about singletons being OK unless you need another one with different behaviour or state but that is not an issue in the applications which I write. In one part of a script I can have the following:

$dbobject =& singleton::getInstance('some _table');
$data - $dbobject->getData($where);

In another part of the script I can have the following:

$dbobject =& singleton::getInstance('some _table');
$data - $dbobject->insertRecord($data);

In this example the state of the ‘some_table’ object is ignored in the second call, and the behaviour is irrelevant as I am using a totally different method.

Some of the so-called “issues” in those articles are down to the inappropriate use of singletons, or because they are in compiled languages whose application state persists across multiple user transactions. With PHP all application state is dropped from memory when the script terminates, so this is not an issue. And as I would not class my use of singletons as “inappropriate” I do not encounter the issues described in those articles. So, if I don’t have any problems with my use of singletons, why on earth should I implement the solution of NOT using singletons?

[TomB]

So, if I don’t have any problems with my use of singletons, why on earth should I implement the solution of NOT using singletons?

Fixed:

So, if I don’t understand the problems with my use of singletons, why on earth should I implement the solution of NOT using singletons?

I’m not going into any more detail as it’s futile. However for anyone reading this who is interested, do check out the links I provided as they explain the problems more than sufficiently.

Again, nobody is telling you not to use singletons. Our argument is not to encourage others to use them. If you want to pollute your own codebase with bad practices, well that’s your problem, but coming here and arguing that it’s not a problem is not helpful for anyone.

[s_molinari]

Just need to remove the word “have” in your fix there Tom.

Scott

[TomB]

well spotted!

[cpradio]

Okay guys, we deviated again.

Let’s make this really basic this time (no personal feelings involved!).

What are the advantages to using DI? What are the disadvantages? When should you use it and when should you not?

We’ve seen that in cases where the implementation can be convoluted, we can use a Factory pattern to continue the usage of DI (thanks @TomB). We’ve also seen that singletons can take that place too (right or wrong – who cares!).

We’ve seen that DI has helped with writing tests and using TDD practices. We’ve also seen that DI helps you write your code with separation of responsibility front and center (otherwise, your code becomes convoluted and you start to see yourself mixing practices).

What else can we identify? Again, no personal anecdotes. Simply speak on technical terms. I don’t care if you don’t believe in DI, that doesn’t help the discussion, what does matter is how you technically implemented something without DI. The how is important, not necessarily the why.

Make sense? Good. GO!

[TomB]

What else can we identify? Again, no personal anecdotes. Simply speak on technical terms. I don’t care if you don’t believe in DI, that doesn’t help the discussion, what does matter is how you technically implemented something without DI. The how is important, not necessarily the why.

Can I add to this that examples need to be minimal and complete (e.g. all the code required to run them) and not need an understanding of a large underpinning framework or codebase. As we’re discussing concepts not implementations, using specific implementations as examples to explain the concepts is not helpful as it mixes implementation details with the underlying concepts.

[cpradio]

One last rule to ensure the discussion hopefully stays on track. If a statement is made and you want to discuss the merits of that statement further. Please use the Reply as linked Topic option under the given post.

This permits 2 things, 1) it lets this discussion continue to focus directly on the benefits or lack there of of DI, and 2) lets you put a focus on a another path by “diverging the discussion” into its own topic.

It is one of the best features of Discourse (with the exception to the wording it uses… who thinks of “linked topic”?).

Thanks all.

[tony_marston404]

@TomB

You cannot understand the benefits of DI yet you’re coming here arguing that it doesn’t have any benefits.

Wrong. I understand that DI has benefits in CERTAIN circumstances but that does not mean in ALL circumstances. If you look at http://www.tonymarston.net/php-mysql/dependency-injection-is-evil.html#what.is.the.problem you will see that I acknowledge Robert C. Martin’s COPY module as a case where DI is a good idea. I have explained in this thread that I use a form of DI to inject view names and model names into my age controllers. I even use it to inject my models into the views so that the view can extract the data from whatever model it is given. But what I object to is the argument that I MUST use DI in EVERY place where I have a dependency even though, in my humble opinion, the only result would be to make my code more complex and therefore less maintainable. I object most strongly to this dictatorial attitude, and all the while I am being attacked for having a different opinion I will exercise the right to defend myself.

@TomB

providing no sources and code examples that uses known bad practices is detrimental to others.

I have provided a code sample in an earlier post where I use a hardcoded dependency, and everybody has told that this is wrong. As for “known bad practices” the fact that some groups of programmers consider them to be bad is irrelevant as there are other groups of programmers who hold a different opinion. There is no “best practices” document which is universally accepted by all programmers, so please stop telling me that just because I don’t do what you do that I am automatically wrong.

[TomB]

@tony_marston404

I have provided a code sample in an earlier post where I use a hardcoded dependency, and everybody has told that this is wrong. As for “known bad practices” the fact that some groups of programmers consider them to be bad is irrelevant as there are other groups of programmers who hold a different opinion. There is no “best practices” document which is universally accepted by all programmers, so please stop telling me that just because I don’t do what you do that I am automatically wrong.

Yes, a group of academics, professional programmers who work for IBM, Google, Microsoft, and the majority of people with an interest in programming vs a handful of bloggers who are at nowhere near the same level. There is such a thing as “best practices” to argue otherwise is ludicrous.

“Best” is not as subjective as you’re pretending it is. Saying “Y causes X, X is an undesirable trait therefore Y is bad” is not subjective. Just because you cannot see why something is undesirable does not mean it isn’t.

This discussion is over until you provide short and to the point code examples (That don’t need an understanding of your own framework) of where another solution is better than DI. The merits of DI are widely established, it’s up to you to show otherwise and you have repeatedly failed to do so.

Fact: Singletons heavily limit flexibility as you admit (your argument is that you do not need that flexibility)
Fact: Singletons require more code and have worse performance than DI (See above)

Based on the above: Even taking your claim that you do not need the flexibility that DI offers over singletons as true, the conclusion is that in this circumstance DI and Singletons are equally beneficial. Therefore, anyone with a logical mind would pick the one that was quickest to code, had best performance and may have some benefit in the future which on all counts is DI.

[tony_marston404]

@TomB

tony_marston404:
and in the process it introduces more complexity, more levels of indirection, more code to execute, more code to read and understand, and the potential for more bugs.

This is a flat out lie.

I disagree. Take this sample of my code where I have a hardcoded dependency

class Person {
    function doStuff () {
        ....
        $objAddress =& singleton::getInstance('address');
        $address = $objAddress->getPrimaryAddress($customer_id);
        ....
    } // doStuff
} // end class Person

If I were to follow the “best practices” that have been described in this forum then I would need to have code situated in a number of different places:

1. In a dependency config file where I can identify that ‘address’ is a
   dependency of ‘person’.
2. In the place where the ‘person’ object is
   instantiated so that it can also instantiate the ‘address’ object
   and inject it into the ‘person’ object.
3. In the ‘person’ class where I would have to define a property to hold the injected object.
4. In the place where the dependency is consumed as I would have to switch from using a local variable to a class property.

Although #4 would result in one less line of code in that particular method, I now have other lines of code which need to exist in three other places. This means more lines of code in multiple places instead of my existing two lines of code in a single place. More lines of code means more lines to read and more lines to execute. Anybody looking at my original code will understand what is going on immediately without the need to look anywhere else (except, perhaps, to examine the getPrimaryAddress() method in the ‘address’ class).

With the DI option all I can see is a method being called on a class property (which means that it should be an object and not a scalar) which means that I now have to look somewhere else in order to identify exactly which class was instantiated into that particular object. Because this information is now held in multiple locations it will take more time to read and therefore more time to understand. This is what I mean by “more levels of indirection”. More lines of code also increases the potential for more bugs. The fact that I now have more lines of code in several places instead of my two lines of code in a single place most emphatically make the DI option more complicated and more expensive.

When you consider the fact that I will NEVER need to change that dependency in that class you should also consider my reluctance to make my code more complicated in order to provide a feature that I will never use.

BTW, your example of a separate getInstance() method inside each class is not what I use. That method only exists in a single singleton class.

[TomB]

Please include all the code required to run your example including the singleton::getInstance method and the constructor of the address object.

[TomB]

When you take into account the code required by the singleton object, then yes, it’s more. This is shorter:

class Person {

    function doStuff () {
        ....
        $address = $this->address->getPrimaryAddress($customer_id);
        ....
    } // doStuff
} // end class Person