Dependency Injection Breaks Encapsulation

PHP
1

This is a continuation of the discussion at Dependency Injection: a discussion of the pros and cons which was supposed to focus on the question of DI and the arguments for and against its use, but which degenerated into a series of personal attacks. I would like to see if it is possible to have a civilized debate on this subject, and I have decided to stoke the fires a little bit by saying that, in my humble opinion, not only are there circumstances where Dependency Injection (DI) is not a good idea, but if it is employed in those circumstances it has a nasty side effect in that it actually breaks encapsulation. Before you say that this is a stupid idea, let me say that it is no more stupid than the idea that Inheritance breaks encapsulation (refer to http://en.wikipedia.org/wiki/Information_hiding#Relation_to_object-oriented_programming).

In the article at http://www.loosecouplings.com/2011/01/how-to-write-testable-code-overview.html the author identifies three categories for classes: value objects, entities and services and explains why each of these categories can be treated as either “injectable” or “newable”. Value objects and entities are newable while services are injectable.

My framework is an implementation of the 3-Tier Architecture (see http://www.tonymarston.net/php-mysql/3-tier-architecture.html) combined with the Mode-View-Controller design pattern (see http://www.tonymarston.net/php-mysql/model-view-controller.html). In this structure all the Model classes, one for each database table, qualify as “entities” while the Controllers, Views and Data Access Objects qualify as “services”.

There are places in my framework where I have found a use for DI, such as injecting my Models into my Controllers and Views, but where I have a Model class which is dependent on another Model class I do not use DI. This follows the loosecouplings rule that “entities” are not “injectable”.
This means that code such as the following:

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

Is perfectly valid as it stands because both the ‘Person’ and ‘Address’ objects are entities and therefore not injectable.

Although I could employ DI here I do not see the point in doing so as it would be a complete waste of effort. I do not need the ability to switch the ‘Address’ object to another object for the simple reason that what I want at that moment in time is an address, and that can only come from the ‘Address’ object. The ability to switch to another object would be a pointless as it could never supply an address.

The idea that DI breaks encapsulation comes from the description of implementation hiding which states

In other words, a class exposes its methods, but the code behind those methods should be hidden. This means that if the implementation requires the use of other dependent objects then the caller should not need to specify which objects to use. It should be possible to change the implementation without having any effect on the calling object.

So, there are two points to be discussed:

  1. Are there circumstances where using DI would not provide any benefits and therefore should not be used?
  2. Are there circumstances where using Dependency Injection breaks encapsulation?

This is just a summary of the information contained in my article http://www.tonymarston.net/php-mysql/dependency-injection-is-evil.html

@Hall_of_Famer, @swader, @Jeff_Mott, @s_molinari, @fretburner, @cpradio, @DaveMaxwell

2

[Citation needed]

Please provide a code example that demonstrates a situation where DI breaks encapsulation.

Again, your arguments are all “MY CODE CANT”, “MY CODE DOES THIS” I thought we’d already established in the last thread that this is not a sensible method of discourse as it is highly reliant on understanding a much larger system and does not allow for conceptual discussions of the individual topic at hand.

Any mentions of “Your framework” are entirely irrelevant so please just stop.

If you really do want to have a sensible discussion you need to be able to decouple your points from your existing codebase and provide simple, short, to the point code examples that demonstrate the concept you are trying to convey. Ideally two examples: One that has the problem you suggest, and another that does not so we can compare and analyse the differences.

edit: Once again you’re using the exact same example as before. Who are you trying to justify this code to? It seems like it’s yourself. Nobody is ever going to agree that a singleton is the better approach here. Once again I’l leave you a list of sources to go away and read:

http://blogs.msdn.com/b/scottdensmore/archive/2004/05/25/140827.aspx
http://crazycoders.net/from-tight-coupling-to-dependency-inversion-stupid-series/
http://codebetter.com/jeremymiller/2009/01/06/it-s-been-years-since-i-ve-gone-on-an-anti-singleton-rant/
https://nikic.github.io/2011/12/27/Dont-be-STUPID-GRASP-SOLID.html
http://misko.hevery.com/2008/08/17/singletons-are-pathological-liars/
http://misko.hevery.com/code-reviewers-guide/flaw-brittle-global-state-singletons/

1 Like
3

I think that’s true. If I want to use class X, should I know or care whether it relies on another library? That’s all part of the implementation, right? And yes, in fact, that’s true.

But there are trade-offs.

The first is: How does class X get its dependencies? For example, if I were to take one of your classes, Tony, and try to use it in one of my projects, it wouldn’t work, and one of the most fundamental reasons is because your code requires your singleton class to manage dependencies. That hard dependency on the service container is especially bad because the container has to manage dependencies not just for the classes you wrote, but for the entire application.

Imagine if the classes I wrote used my container, and the classes you wrote used your container, and the classes that everybody else wrote each use their own respective container… then no dependency could ever be shared. None of us would ever be able to share code, and we’d all be forced to keep re-inventing the wheel, because every one of us would be creating wheels that only work on our own personal car.

The ideal is for all our classes to be ignorant and independent of the service container. But if your code were ignorant of the singleton class, then we’re back to the question of how do you get dependencies? Almost every choice requires hard-coded knowledge of the larger application. The only way you could get your dependencies in a way that’s portable and flexible is through constructor arguments.

At this point, I presume your response would be something along the lines of, “I don’t have that problem. I don’t use other people’s code. I don’t care if they can’t use mine.” And that certainly all seems true. All I can say is, that kind of approach leads to code that only works with itself and is therefore widely considered to be bad. The consensus seems to be that good code plays nice with others.

If you’re happy on your island, then… fine. But don’t make the mistake of thinking that’s considered the best way to do things.

4 Likes
4

Great post, but I’m not sure I understand this. How does that break encapsulation? if I have $foo->bar() and the bar() implementation uses other objects internally, that’s hidden from the calling code, the essence of encapsulation. Perhaps I’m missing something here but I can’t see how that breaks encapsulation. $foo->bar()->baz() clearly does, but that’s more specifically a Law Of Demeter violation.

5

If you focus on only that one method, then sure. But the idea of encapsulation is usually applied to the entire class, including its constructor and arguments.

6

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

7

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
8

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
9

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
10

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.

11

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://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
12

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.

13

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.

14

$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.

15

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.

16

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.

17

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
http://devblog.avdi.org/2011/07/05/demeter-its-not-just-a-good-idea-its-the-law/

18

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.

19

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:

20

Kobayashi Maru… that’s all I have to say.

6 Likes