Dependency Injection Breaks Encapsulation

[quote]- DI is inappropriate when a dependent object can only be instantiated from a single class where there are absolutely no alternatives.
[/quote]

Why? What makes it “inappropriate”. This is an baseless assertion. Why is DI inappropriate in this scenario?

edit: More importantly, what makes a singleton “more appropriate”?

I am using a singleton instead of DI in those places where the circumstances for which DI was designed do not exist. I could easily have used the “new” verb, but as the same class could be instantiated several times it is more efficient to instantiate it just once and use the same instance in multiple places.

“I chose not to” is not a valid argument and doesn’t explain why a singleton is better than DI in this case.

This is a red herring. The singleton class adds its own overhead. the result of this is that:

  1. When the condition is not met and the class is not used, the singleton version is faster
  2. When the condition is met (e.g. is valid) and the class is passed in with DI then the DI version is faster
  3. If the lazy loaded class is already used somewhere else in the script (likely) then all you’re doing is adding the overhead of the singleton for no advantage.

Let’s bench it:

<?php
$t1  = microtime(true);

class X {

}


for ($i = 0; $i < 1000000; $i++) {
	new X();	
}



$t2 = microtime(true);

echo $t2 - $t1;

0.11410284042358

<?php
$t1  = microtime(true);

class singleton {
	private static $instances = [];

	public static function getInstance($name) {
		if (!isset(self::$instances[$name])) {
			self::$instances[$name] = new $name;
		}

		return self::$instances[$name];
	}
}
class X {

}


for ($i = 0; $i < 1000000; $i++) {
	singleton::getInstance('X');
}



$t2 = microtime(true);

echo $t2 - $t1;

0.27273297309875

And this isn’t even a fair test. In reality, the singleton isset condition would evaluate to false once per request, so this is more fair for this test:


<?php
$t1  = microtime(true);

class singleton {
	private static $instances = [];

	public static function getInstance($name) {
		if (true) {
			self::$instances[$name] = new $name;
		}

		return self::$instances[$name];
	}
}
class X {

}


for ($i = 0; $i < 1000000; $i++) {
	singleton::getInstance('X');
}



$t2 = microtime(true);

echo $t2 - $t1;

which gives 0.41565203666687.

So the singleton is unsurprisingly slower. In this case 4 times slower. For the singleton version to offer an average benefit in performance then:

  • The condition must not be met at least 80% of the time
  • and nowhere else in the code must be constructing the dependency. If other places are instantiating the dependency then you get the overhead twice or more… which is inarguably slower.

No it isn’t. That is like saying that a purr method and a fly method and an open method are more readable all attached to the same class than if purr were attached to a cat class, fly to an aircraft class and open to a door class. With all of them on the same class you imply that a cat can open a door can fly and an aircraft can purr.

2 Likes

Hmm… the definitions I know of are a bit different or rather I understand them differently. The definition of encapsulation I know of is about the hiding of data or methods in order to simplify a client’s usage of the objects or modules in question.


http://www.adobe.com/devnet/actionscript/learning/oop-concepts/encapsulation.html

The last one offers the closest reasoning to yours, but notice in his example.between the position class and the route class. Why didn’t Mr. Rogers just add the position methods into the route class? That would follow your understanding of encapsulation. Why he doesn’t do it? It is in his “2nd rule of encapsulation”, which is.

Encapsulation rule 2: Use responsibility-driven design to determine the grouping of data and operations into classes

which is the rule you ignore to make your 9000 line monster class viable in your mind. It is wrong. It is bad coding. It is hard to understand (which readability is all about). It basically kills all your arguments about DI.

No, you take issue with those who say it is ok to use it everywhere. The appropriate is a given (and why I agreed with you.) You are adding the “inappropriate and superfluous” to the equation and then you pull it into your world of programming to say, most of the time it is inappropriate and superfluous and that is YOUR issue. As Tom is trying to get you to explain to us, what is your definition of “appropriate”? If it is based on your own code, it thus shows basically NOTHING about what appropriate means. Your now and then usage of DI is NOT appropriate, because the basis your code is built on is NOT appropriate. You have absolutely no grounds to judge what appropriate is, because you don’t know what appropriate means. You DO NOT follow the second rule of encapsulation.

Encapsulation rule 2: Use responsibility-driven design to determine the grouping of data and operations into classes

Scott

1 Like

You are missing the point, as usual. All those methods are contained within my abstract table class and are inherited by each concrete table class. Those methods cover everything that I may wish to do with a database table, so it is perfectly valid that they belong in the same class.

I asked this before to no answer, what do “popups”, “workflows”, “buttons”, “PDFs”, "CSVs "and the various other responsibilities have to do with “everything that I may wish to do with a database table”, They are nothing to do with a database table, they are disparate concepts entirely.

As an aside, I honestly cannot believe we’re having a discussion about whether a class that has 100+ variables breaks SRP or not, it’s self-evident. There’s simply no room for debate here. The answer is either: “Yes it does” or “I don’t understand what a responsibility is”.

Let’s break it down:

Database I/O is a responsibility I agree. However that’s not the same responsibility as generating a PDF from some data in the database or handling a file upload, processing the file contents and then inserting the data into a database. You seem to be following the flawed train of thought: “The application needs to generate CSV files and PDF files and create popups, they are all responsibilities of the application therefore all belong in one class” which is so backwards I don’t even know where to begin.

The most important part is the first sentence in that wikipedia definition:

The idea of selective hiding of properties or methods is an optional extra that came later. I should also point out that encapsulation means “implementation hiding” and not “information hiding”.

Here you are talking about the Single Responsibility Principle (SRP), but as my framework is built around a combination of the 3-Tier Architecture and the MVC design pattern it has all the separation it needs. I have 300+ Model classes which deal with all application logic, I have separate reusable Controllers for each Transaction Pattern, I have separate reusable View components for HTML, CSV and PDF output, and I have separate components for talking to the database.

If you agree with me that DI is useful “when appropriate” then you must also agree that the inverse is also true - that DI is NOT useful “when NOT appropriate”.

Let me say this yet again - DI is appropriate when a dependent object can be supplied from a collection of alternative classes, and that is exactly how I use it in my framework. DI is NOT appropriate when a dependent object can only come from a single class where there are no alternatives, and in that situation I do NOT use DI in my framework.

I object most strongly to those who keep telling me that I should ALWAYS use DI in ALL circumstances, even when those circumstances fall outside the objectives of DI as described by Robert C. Martin in his article at http://www.objectmentor.com/resources/articles/dip.pdf. If you bothered to understand the purpose of DI you would see that it is only beneficial when a dependent object can be supplied from more than one class. So when the choice is NOT more than one class then DI is NOT appropriate.

I repeat, I use DI in my framework in all those cases where a dependent object can be supplied from multiple classes, but where a dependent object can only come from a single class I do not use DI. Whether or not DI is appropriate boils down to a simple question - how many alternative classes can be used to supply a dependent object. If the answer is “more than one” then DI is appropriate.

BTW, DI has absolutely nothing to do with encapsulation.

[citation needed]

Please explain why this is a measure of “Appropriateness”. By what measure is a singleton more “Appropriate” than DI when you only have one option?

I asked you a very simple yes/no question and yet again you chose to avoid it. You are incapable giving a straight answer to a question. Please answer the following:

  1. How are you measuring appropriateness?
  2. Do you agree my statement in post #309 where I asked “Are, in your mind, correct? my second example using the singleton class is what you’re using in place of DI? Please answer with a simple yes or no.” It’s a simple yes/no question yet you’re incapable of answering it.
  3. Do you understand the demonstrable fact that DI can be used any place you can use a singleton?

Each time you are asked a direct question you avoid it for 50+ posts and then claim you answered it before. Please stop the games.

You keep using the word “Appropriate”, then you give a definition that doesn’t answer how you’re measuring it and keep using it. I’ll just post this again:

edit: To clarify you can’t just state “Bikes are more appropriate than cars when there is only one passenger” You need to use a metric to explain why you’re making that assertion. Speed? MPG? Some definable, measurable metric for how you can actually quantify “appropriateness” Blanket statements like Whether or not DI is appropriate boils down to a simple question - how many alternative classes can be used to supply a dependent object. If the answer is “more than one” then DI is appropriate.Whether or not DI is appropriate boils down to a simple question - how many alternative classes can be used to supply a dependent object. If the answer is “more than one” then DI is appropriate. are meaningless

If they were all specific to a database access class (and therefore all contained code for accessing a database) then you would be correct but most of the methods you have there are as much to do with databases as a purr method would have.

The main purpose of classes is to break up the code into manageable pieces where you don’t need to look at other classes in order to understand what the overall code is doing. A class containing several hundred lines of code is generally becoming too big to be manageable and so at that point you ought to be looking at the hierarchy of what that class consists of so as to create several classes defining component classes that can then be combined together to define the original class using a much smaller number of lines of code that will be far more readable.

Having more than say 500 lines of code in a single class means that the project has most likely not been defined correctly as if a class needs that many lines of code then it should almost certainly be defined in terms of several classes at the next level down within the hierarchy in order that it can be properly understood and maintained.

Defining everything in a huge monolithic class is not using object oriented coding - ot is the modern equivalent of spagetti coding …

I would consider any class having over 500 lines to be invalid simply because it is trying to do too much iin the one class and hasn’t been broken down to a low enough level.

2 Likes

Tony,

Again, you have no argumentative foot to stand on, when it comes to defining anything as being “appropriate” or “inappropriate”, because you don’t know what they mean in terms of OOP programming. Your 9000 line monster class is the proof. Your defending it only makes you look less and less professional. It is wrong. It is poor programming. It isn’t even outdated or archaic, because no able OOP programmer would program like that to begin with, not even 30 years ago. Not only Tom or I have noted this, there are a few others who have too now. Yet, you still continue to argue you know better? You still argue a 9000 line class is proper and even good?

You know what your framework looks like to me? It looks like you were relatively good at PHP’s procedural programming paradigm with PHP4, then decided to try and make your framework work with PHP5 and OOP, but only with your old procedural knowledge and not really with a proper understanding of OOP. You worked incredibly hard to put your framework together with what you thought was OOP, but instead of properly learning OOP, you just continued to do bad practice, after bad practice. And instead of learning the right way to do OOP and/ or leaving your mind open to that paradigm the whole time, you started to declare everyone else as the “idiots”, because they don’t follow your poor methodologies.

You need to stop, take a step back, and look at your work and be totally honest with yourself. What you’ve done is fantastic, because you worked hard on it and were very determined to make it work, which is something I actually admire. However, the basis is simply wrong. The basis comes from your incorrect perspectives on OOP. Your framework may work. Great. But, the design is NOT good. You simply cannot sit there and go, “My perspective on OOP is right, because it works with my framework.” That is all the argumentation you bring to the table and it simply isn’t enough and why we cannot move forward with this argument/ thread.

DI does not break encapsulation. It indirectly supports it.
9000 line monster classes are a big “no, no” in any terms of OOP.
DI is a great design pattern, used by practically every framework, which aims to be modular and testable in design. It is very appropriate to use, in order to meet those goals.
OOP, at its core, is about being modular in design, which in turn supports reusability of code (and says NO to 9000 line classes!!!) DI supports that core OOP principle. In other words, even a dumb blind idiot monkey programmer would be right in using DI for meeting those goals, which any good framework should try to achieve.

Scott

1 Like

“The most difficult subjects can be explained to the most slow-witted man if he has not formed any idea of them already; but the simplest thing cannot be made clear to the most intelligent man if he is firmly persuaded that he knows already, without a shadow of a doubt what is laid before him.”

-Tolstoy

1 Like

Oh yeah, this thread is still going on? How surprising. If anyone actually notices, someone brought it to Reddit and it was very good read:

3 Likes

LOL! I think we did a much better job here discussing the subject. :wink:

Scott

we certainly covered it in more depth :wink:

1 Like

Though there were some nice quips in there that I found interesting. I haven’t made it through the whole thing, but the first third was a bit interesting.

That whole thread on reddit looks almost like a free-for-all “Whack-a-Tony”. I noticed you threw in a whack yourself there Hall_of_Famer. LOL!

I’d like to think I was (we were?) trying to help Tony, since I was/ am (we were/ are) speaking to him directly here. The fact he most likely doesn’t want my (our) help is the issue I (we) might need to realize?

Maybe the public ridicule on reddit will help?

Or it might turn ol’ Tony into even more of a PHP Scrooge?

Scott

Not always. An error event dispatcher has to get the root dispatcher that PHP will be calling through set_exception_handler, set_error_handler et al up and ready at all times despite the fact that, if all goes well, it will never be called. Usually a symptom of flawed design is more accurate.

lol yeah, it was kinda fun, wasnt it? I honesty think you guys were being too nice to him, if Tony would ever posted such things on Reddit he would get so much ridicule that eventually he would ragequit. The Sitepoint community is a help/support forum, while Reddit PHP is more of just an information exchange/idea forum. I guess this implies the differences. This thread was meant to educate, especially for junior developers who are still confused with the idea of Dependency. I think, theres a reason why this thread was opened again after it had been closed 3 weeks ago.

And I doubt help or ridicule would work on Tony anyway, what he would need is a business failure in which the clients were fed up. But I dont think it would happen, after all Tony was building a legacy framework designed to work for legacy applications. This pretty much dictates that his clients would not have a problem with his framework anyway. Just like if you build a software designed to work for Windows 3.1 or 3.2, the customers will complain how strange it looks on your windows 7 or 8 machine, but the rare ones who still use Windows 3.1 or 3.2 are happy. You are not a person who cant think abstractly, so I know you understand this analogy. XD

Anyway, Tony is more than satisfied with his work, so we should be glad for his accomplishment and there’s little point in helping him, dont you think so?