Dependency Injection Breaks Encapsulation

[tony_marston404]

@TomB

You keep using the word “appropriate” but you have never explained what you mean by that term. Similarly, you have never shown what solution you use in its place.

I suggest you read the article written by Robert C. Martin which I have referred to several times. This describes those circumstances where DI is appropriate, which implies that in other circumstances it is not appropriate.

@TomB

Yes, you do have the problem that DI solves. That problem is locating dependencies.

Where a dependency can be supplied from a collection of alternative objects then I DO use DI. But where a dependency can only be supplied from a single object without the possibility of any alternatives I instantiate the object in the calling module without it being injected. Why can’t you understand such a simple concept?

@TomB

somehow you need to make A know about B.

If my Person module wants to talk to my Address module, then what is wrong with instantiating the Address object right where I need to use it? There is only one Address class that I can use, so DI would be overkill in these circumstances.

@TomB

How does one object know about another? In order to call a method on another object I must have a reference to it in a visible scope

But it all depends on whether the object I wish to use can have may sources or just a single source. If it has many sources I use DI. If it only has a single source I do not use DI. Instead I use code similar to the following inside my Person class:

$addressOBJ =& singleton::getInstance('address');
$data = $addressOBJ->getData($where)

The only difference between using or not using DI is where the class is identified from which the object is instantiated. If there are several alternative possibilities then I use DI to inject the dependency. If there are no alternatives - such as in the above example where there is only one class in my entire application from which I can obtain a postal address - then I prefer to take the simpler non-DI approach and hard-code the reference to the dependency immediately before I need to use it. This technique is called lazy loading (see http://en.wikipedia.org/wiki/Lazy_loading) which is more efficient than your method of eager loading.

[TomB]

@tony_marston404

Why can’t you understand such a simple concept?

Because you don’t seem to realise that DI can be used in any place you are using a singleton. Answer my question about the code examples (with a simple yes/no) and then answer this: What makes the singleton more “appropriate” than DI.

which implies that in other circumstances it is not appropriate.

Implies != demonstrates. Please demonstrate these circumstances with code examples.

which is more efficient than your method of eager loading.

Wrong again, if you provide some code I’ll benchmark it, thinking about it for a split second proves that statement incorrect: Lazy Loading creates more lines of code and therefore a higher execution time.

You’re going to say “Well what if it never gets loaded” to which the answer, as Scott is trying to tell you, is that your application has a flawed design because it breaks single responsibility principle

[tony_marston404]

@s_molinari

There is a rule in good OOP, which says, your classes should have a single responsibility. 9000 lines of code means, that clearly IS NOT happening. Edit: And you totally missed the point of readability. A 9000 line monster class is not exactly fun to read.

Then your definition of the SRP is different from mine. That 9000 line class to which you refer is an abstract class that contains every single method that may be used in any of my concrete table classes. Whether you like it or not every single method in that class is used somewhere in my application, so every method is necessary, and having all those methods in a single class is much more readable than having then scattered across dozens of smaller classes.

The definition of encapsulation is that a class should contain ALL the methods and ALL the properties relevant to the object which it serves, so splitting them across multiple classes would violate the rules of encapsulation.

[quote=“s_molinari, post:310, topic:113596”]

tony_marston404
If there are no alternatives then clearly DI is superfluous.

Ok. This is true.[/quote]
Wait a minute!!! Are you actually agreeing with me??? Well, I’ll go to the foot of our stairs!

@s_molinari

Please now go change your post to say that and change the title and take out the insult to the smart people out there, who use DI properly.

I have no argument with those who use DI properly and in appropriate circumstances, but I do take issue with those who use it everywhere even when it is inappropriate and superfluous.

@s_molinari

Can you also agree that there isn’t a better alternative to DI, when DI is appropriate to use?

I cannot disagree for the simple reason that I regularly use DI in my framework when it is appropriate to do so, but when it is not appropriate then I don’t.

[TomB]

Tony please stop using the word “appropriate” it’s ambiguous and you haven’t defined it. How are you measuring “Appropriateness”?

Weasel word

[tony_marston404]

@TomB

Because you don’t seem to realise that DI can be used in any place you are using a singleton.

Yes, it can, but you seem to be confusing “can” with “must”. I could use DI in those circumstances, but I choose not to because there would be no advantage in doing so.

@TomB

which is more efficient than your method of eager loading.

Wrong again, if you provide some code I’ll benchmark it, thinking about it for a split second proves that statement incorrect: Lazy Loading creates more lines of code and therefore a higher execution time.

Incorrect. Eager loading requires that you instantiate an object in a different part of the code from the place where it is actually used, and if you never run the code which uses it then you have wasted time in creating an object (plus all of its dependencies) which is never used.

Having an object which never gets used is not a symptom of a flawed design. Suppose you have a dependency which is only used when some conditions are met at run-time, but those conditions (such as data validation) are not met. That means that the object you instantiated before evaluating those conditions is thrown away without being used.

[TomB]

@tony_marston404

Yes, it can, but you seem to be confusing “can” with “must”. I could use DI in those circumstances, but I choose not to because there would be no advantage in doing so.

So what is the advantage of using a singleton? What makes your singleton approach more “appropriate” than DI?

In your own words you only use a technique if there is a “benefit”. What is the benefit of a singleton over DI?

[tony_marston404]

@TomB

Tony please stop using the word “appropriate” it’s ambiguous and you haven’t defined it. How are you measuring “Appropriateness”?

I have described in several places what I mean by “appropriate” and “inappropriate” circumstances for the use of DI, and even Scott has eventually agreed with me (see post #310). Just for your benefit I shall explain AGAIN:

• DI is appropriate when a dependent object can be instantiated from a number of alternative classes.
• DI is inappropriate when a dependent object can only be instantiated from a single class where there are absolutely no alternatives.

[TomB]

[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”?

[tony_marston404]

@TomB

So what is the advantage of using a singleton? What makes your singleton approach more “appropriate” than DI?

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.

[TomB]

@tony_marston404

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.

[TomB]

@tony_marston404

Having an object which never gets used is not a symptom of a flawed design. Suppose you have a dependency which is only used when some conditions are met at run-time, but those conditions (such as data validation) are not met. That means that the object you instantiated before evaluating those conditions is thrown away without being used.

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.

[felgall]

@tony_marston404

having all those methods in a single class is much more readable than having then scattered across dozens of smaller classes

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.

[s_molinari]

@tony_marston404

The definition of encapsulation is that a class should contain ALL the methods and ALL the properties relevant to the object which it serves, so splitting them across multiple classes would violate the rules of encapsulation.

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.

Encapsulation (computer programming)

http://www.adobe.com/devnet/actionscript/learning/oop-concepts/encapsulation.html
http://www.javaworld.com/article/2075271/core-java/encapsulation-is-not-information-hiding.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.

@tony_marston404

I do take issue with those who use it everywhere even when it is inappropriate and superfluous.

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

[tony_marston404]

@felgall

tony_marston404
having all those methods in a single class is much more readable than having then scattered across dozens of smaller classes

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.

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.

[TomB]

@tony_marston404

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.

[tony_marston404]

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

en.wikipedia.org
Encapsulation is the packing of data and functions into a single component.

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

@s_molinari

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

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.

@s_molinari

tony_marston404
I do take issue with those who use it everywhere even when it is inappropriate and superfluous.

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.

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.

@s_molinari

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.

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.

[TomB]

@tony_marston404

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.

[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:

Weasel word

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

[felgall]

@tony_marston404

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.

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.

[s_molinari]

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

[TomB]

“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