Dependency Injection Breaks Encapsulation

Can you give a quick example? I can't grasp the issue here.

A couple other reasons off the top of my head are:

Testability: I know you don't care about this one, Tony. All I can say is that it's widely considered to be a good thing. Imagine if the days or weeks spent on manual QA could be automated. You save time and money. Plus, computers make fewer mistakes than humans. Your testing would likely become more consistent and more reliable.

Spooky action at a distance: This was in one of Tom's various references, from Miško Hevery at Google. "Spooky Action at a Distance is when we run one thing that we believe is isolated (since we did not pass any references in) but unexpected interactions and state changes happen in distant locations of the system which we did not tell the object about. ... [Class X] pretends that you can just instantiate it and call a method. But secretly, it collaborates with [class Y]. The [class Y] API says that it can be initialized in isolation, but in reality, it needs the [class Z]. And the [class Z] needs the database. To the developers who wrote this code, it is obvious that [class X] needs [class Y]. They wrote the code that way. But to anyone new on the project, this is a total mystery, and it hinders the learning curve."

I've heard other top engineers say similar things. The consensus seems to be that dependencies should not be encapsulated. A class should make its dependencies obvious.

3 Likes

Oh my goodness, I thought this discussion was over already, wasnt it? Anyway someone banished me from this discussion so I am outta it anyway. There's no need to reference my name in any posts of this topic anymore, although Id be glad to just browse the thread and get some fun and interesting ideas out of it. Jeff Mott made a very excellent post, it was good read.

1 Like

I am going to also answer at a much higher level, since I was personally mentioned and because going down to lower levels means I'll lose the argument anyway. Mark Twain has a good quote on that, which I won't quote, as it gets too personal and I notice getting too personal means posts here get deleted directly without notice, warning or reason.

Tony, your framework might work and that is great. But, just because it works, it doesn't mean it presents a more correct way to do things or said differently, it doesn't mean the standards for PHP programming used by the majority of (good) developers today are wrong (or evil). Please stop trying to justify your ways of doing things as "more right", because, they simply aren't. Why aren't they? Because what you do is done by, at best, a minority of programmers today. I'll even go so far as to say a minority of unprofessional programmers. Sorry if that is too personal, but these discussions are getting old.

Proper DI is only evil, as it sounds, because it doesn't work within your framework or you don't want to accept it could help a lot with modernizing your framework. Does that mean it is bad or wrong or evil? Absolutely not. The same goes for your comment in the other thread, which was something like, "I don't do unit testing, because I don't need to. That is what QA is for." What does that say? Your framework and your development attitude are both antiquated or let's say "less correct" according to today's written and unwritten coding standards.

My friendly suggestion. Get up-to-date Tony. You'll do a lot better for it and we won't be wasting our time with these unneeded threads.

To answer your questions at a higher level too. Yes, DI shouldn't be used everywhere and there is tons of information about when and when not to use it. You also made some good points actually. However, the argument isn't about when or when not to use it. The argument is, is or is it not a very beneficial design pattern, when used properly? And the answer is, an absolutely definitive friggin -> YES, IT IS!!!!

Scott

1 Like

Take the following code sample:

$objPerson = new Person();
$result = $objPerson>getData("person_id=1234");

Here I am asking for data for a particular person. Some of this data may come from other tables, such as the Address table, but I don't need to specify the identity of any of these other tables. This is dealt with in my Person class with code similar to the following:

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

Using DI I would have to instantiate and inject $objAddress into the Person object using code similar to the following:

$objPerson = new Person(new Address);

Bearing in mind that address data can ONLY be obtained from the Address object, this means that the Address object is not actually interchangeable with any other object, so is not a viable candidate for DI in the first place.

Encapsulation is the process of placing all an entity's properties and methods in the same class, and the concept of implementation hiding means that while the method signatures are known to the outside world the implementation - the code behind the method - is completely unknown. This is the equivalent of saying "perform this operation, and I don't care how you do it". This also allows the implementation to be changed at any time without affecting the calling module.

By implementing DI I am changing this to "perform this operation, and use this other object in your implementation". This then exposes the implementation to the outside world, and by exposing the implementation you are violating encapsulation.

This may seem crazy to you, but why is it more crazy than the idea that inheritance breaks encapsulation?

If you bothered to read my article (which I have amended slightly, BTW) you will see that it contains examples of where I DO use DI and explains the reasons why, as well as identifying the places where I DO NOT use DI with explanations as to why not. Using examples from my own framework is therefore completely relevant. It would be a waste of time giving code samples that never actually existed in my framework.

This either makes no sense or you do not understand DI. I cannot tell which.

$objPerson = new Person(new Address);

In what way is this "not interchangeable"? It's very interchangable...

$objPerson = new Person(new Address);
$objPerson = new Person(new BusinessAddress);
$objPerson = new Person(new POBox);

In this example with DI when I call $objPerson->getData(), if the address object is used it's entirely polymorphic. Using your singleton class it is not.

You still haven't explained how this violates encapsulation: $objPerson = new Person(new Address); In this example the Person object encapsulates the address object. Saying this breaks encapsulation is factually incorrect.

This may seem crazy to you, but why is it more crazy than the idea that inheritance breaks encapsulation?

This is well documented in our field and all you're doing here is showing your ignorance. Please read the very highly regarded Gang of Four Design Pattens book. Alternatively, here are a few articles you may find interesting:

http://www.javaworld.com/article/2073649/core-java/why-extends-is-evil.html
http://blog.berniesumption.com/software/inheritance-is-evil-and-must-be-destroyed/
http://blogs.perl.org/users/sid_burn/2014/03/inheritance-is-bad-code-reuse-part-1.html
http://www.developer.com/design/article.php/3525076/Encapsulation-vs-Inheritance.htm
http://users.csc.calpoly.edu/~jdalbey/305/Lectures/ExtendsIsEvil.html

By implementing DI I am changing this to "perform this operation, and use this other object in your implementation". This then exposes the implementation to the outside world, and by exposing the implementation you are violating encapsulation.

The "outside" world you talk of is a much higher level of the application. The person object must be constructed somewhere. Is this true of all constructor arguments? How about method arguments? If the above "breaks encapsulation" so does $objPerson>getData("person_id=1234"); because the calling code knows about the argument. Knowing that something is required, is not the same as knowing how it is used.

You clearly have a fundamental misunderstanding of encapsulation. As we went through in the last thread, please go away and read the related literature before coming here and offering your own skewed interpretation of these concepts or provide references to back up your claims. You repeatedly fail to produce working, minimal code examples or references and this is why people struggle to take your posts seriously.

It would be a waste of time giving code samples that never actually existed in my framework

No it would not. As it would be unbiased, decoupled for anything else, minimal and easier to follow.

1 Like

If you read my article you will see that I specifically stated that I do not write class libraries where developers can pick and choose which classes they want to use. I write complete applications, and the Person class is part of my "Party" subsystem which you either use in its entirety or do not use at all.

It is not correct to say that my singleton class "manages" any dependencies. It simply returns an instance of the named class and the name of that class is hard-coded in the object which wants to use the dependency.

I repeat, I do not write sharable class libraries, I write complete applications. You either use the whole application or you don't use it at all.

That is only relevant if my intention is to write code that can be shared outside of my framework. It is not, and I am not going to spend any time in providing a mechanism to do something which I know that I will never do. This would violate the YAGNI principle,

It is also not the "best way to do things" if you implement DI in circumstances where it was not designed to be used. My article contains the following description:

In the above example the Address object is not interchangeable with any other object as address data can only be obtained from a single place, so if it is not interchangeable it is not a good candidate for DI.

The problem is, Tony, that you are coming here and.... sharing the code outside of your framework. So yes, it is entirely relevant and you missed the point of everything Jeff Mott said.

$foo->bar() is simply calling the bar() method on the $foo object, and the fact that the bar() method may need to access other objects in the system is totally hidden. This is a prime example of implementation hiding.

OTOH if you are forced to inject the dependent object(s) into the $foo object, such as with

$foo->bar($dependency1, $dependency2, ...);

you are effectively exposing the implementation to the outside world by telling the bar() method which objects to use internally. This then means that the implementation is no longer hidden, and as implementation hiding is supposed to be a part of encapsulation you have therefore broken encapsulation.

But you miss the point of DI again.

If I have a piece of code that looks like this:

class Foo {
	private $bar;

	public function __construct(Bar $bar) {
		$this->bar = $bar;
	}

	public function doStuff() {
		$this->bar->x();
	}
}

$this->bar->x() can use a dozen dependencies behind the scenes, it simply does not matter if they're there or not to code in the Foo class. I am given a fully constructed Bar object with any dependencies it may or may not need and I don't need to worry about locating them. The Foo class knowns nothing about the implementation of the Bar class. This is the fundamental principal of encapsulation.

Encapsulation applies to all the methods and properties within a class. Implementation hiding applies to each and every method. You are supposed to now what a method does, but not how it does it.

Putting this another way, using your "singleton":

class Foo {
	private $bar;

	public function __construct() {
		$this->bar = singleton::getInsance('Bar');
	}

	public function doStuff() {
		$this->bar->x();
	}
}

The foo class now knows about the implementation of the singleton::getInstance method (it knows it returns a Bar instance). It also knows about the implementation of the Bar instance. (That it has a method called X). This is known as a law of demeter violation. You may find the following links helpful:

http://misko.hevery.com/code-reviewers-guide/flaw-digging-into-collaborators/
http://alvinalexander.com/java/java-law-of-demeter-java-examples
http://c2.com/cgi/wiki?LawOfDemeterExample
http://javarevisited.blogspot.co.uk/2014/05/law-of-demeter-example-in-java.html

While I agree that all that manual QA could be automated, what would be the cost? Apart from the fact I would have to learn a whole new set of testing tools, I would then have to take the time to write all the tests, and bearing in mind that it usually takes longer to write tests than it does to write the code being tested, this would require several years of someone's time. I'm afraid that I only have a budget and timescales for development and maintenance, so if someone wants me to set up a fully automated QA department then they will have to pay for it in both time and money.

To somebody new to a project, not just mine but anybody's, everything will be a total mystery. The advantage of my approach is that the dependencies of any Model object (in order to differentiate between Controllers and Views) are hard-coded within each object so spotting any dependencies will be quick and easy. If you use DI, with or without a DI Container, you end up with more places to look and more time spent looing.

That is only a consensus among those who thin that DI should be employed with every dependency without exception. I do not follow this view. I will only use DI where it is appropriate, such as when an consumer can use a number of interchangeable dependencies, as was illustrated with Robert C Martin's "Copy" program.

Me Me Me Me Me. Yet again you are unable to discuss things conceptually. We are discussing the merits of different approaches and all you can say is "My codebase cannot do that". Abstract thinking is a fundamental skill for a programmer, being able to take a step back and look at the bigger picture regardless of past developments is a useful skill. I posted this is the last topic but I think it's worth repeating:

this is also relevant:

Kobayashi Maru.. that's all I have to say.

6 Likes

This again shows an antiquated attitude. Good PHP developers today build bundles/ packages/ libraries/ components, etc. (and then applications), so that the bundles/ packages/ libraries/ components can be reusable for others and for themselves.

A good example I can come up with quickly is the httpkernel component created for the Symfony framework. This component is a core component for other frameworks too, like Silex and Laravel or for applications like Drupal. If Fabien Potencier, the Godfather of Symfony, had your attitude, then probably Symfony wouldn't be the popular framework it is and none of the work put into it could be reused elsewhere. I am glad most people don't share your views Tony, otherwise my project would be 1000 times harder to do and I'd have to come up with doing the same thing others have already done, again. You know, the proverbial "reinventing the wheel". This was a real issue for PHP, as a programming language, many years ago. Those days are over, thank God and thank Composer and great open minds working with the language today. Because of them, we are a lot further. I would bet, if you decided to rewrite your framework from scratch today, you could put it together in a fraction of the time it took you way back when, all because people like Fabien write reusable code first......then applications.

Scott

The problem here is that Tony is stuck in a cyclic argument. He is basically making these statements repeatedly:

  • My code is my business, stop talking about it I'm not interested in changing it
  • Here's my code, this is how things should be done.

When someone critiques point 2 he responds with point 1.

5 Likes

Agreed. This is also a great example when a "like", as a post rating, just doesn't do the post justice. smiley

Scott

I have never said that the methods used in my framework are "more correct", merely that in achieving the objectives of OOP - to increase code reuse and decrease code maintenance - I have used methods which are different. The reason that I keep quoting samples from my framework is to provide code samples from real life situations.

In my article at http://www.tonymarston.net/php-mysql/dependency-injection-is-evil.html I point to the description of DI in Robert C Martin's "copy" program (see http://www.objectmentor.com/resources/articles/dip.pdf) from which I derive a description where I see DI as being beneficial:

I then show those places in my framework where I employ DI simply because the above description is applicable. I also show places where I could use DI but have chosen not to do so because the conditions in that description are not met.

Your use of "proper" DI implies that there is also "improper" DI. Is this correct?

I never said that I never use DI in my framework, only that I don't use it in those places where I consider it to be inappropriate and of no benefit.

My point exactly! It is beneficial when used properly which implies that it is not beneficial when used improperly. All I am doing is trying to define what circumstances are appropriate for DI and what circumstances are not.

Let me turn the question around. Under what circumstances do you think that DI is NOT an appropriate solution?

This is the exact question I asked you at least a dozen times in the last thread and you failed to provide any example code. You are the one making this assertion but you have still failed to back it up with anything meaningful.

Edit: To answer this, it is well documented. But the answer is not, as you're hoping "Don't use DI". DI Can be misused... see here: https://r.je/oop-courier-anti-pattern.html and http://misko.hevery.com/code-reviewers-guide/flaw-digging-into-collaborators/ the answer is "Only inject things you you really need", it's not "Don't use DI"