Dependency Injection Breaks Encapsulation

PHP
163

Is this actually a joke?

“Microsoft word is a program that allows you to perform any amendment to a document”

“Any operation”?

And what about all those other things:

  • PDF files
  • Time zones
  • CSV files
  • Something about version control
  • Relationships to other objects
  • clearScrollArray??? Scrolling? Some sort of pagination maybe?
  • and workflows
  • File uploads
  • customButton()? This is something to do with the GUI?
  • Array to string conversion (wtf i don’t even, seems to be some sort of serialization)
  • initialiseFilePicker() :? what does that have to do with a database operation.

edit: And why are there erase* and delete* methods. That is painfully opaque.

You have not summed up what the class does, you have vaguely described pretty much anything. This could apply to Wikipedia, Wordpress and anything which is interacting with a database in some way. It may be a summary of an application, but if it’s the summary for your class then clearly the class is doing things that the whole application should be doing.

sigh Ok let’s take this one to vote. Who thinks tony’s 9000 line class is a god class?

164

Me. It doesn’t serve a single responsibility, it serves multiple, which is by definition is a god object. I also object to him classifying it as an “abstract class”. I write abstract classes daily, that is not an abstract class. It is a base class that gets inherited by all other classes to provide various low level operations – it isn’t defining what implementations are permitted, it is defining the actual implementation and you’d have to override anything you don’t want (or want to work differently).

Abstract Class definition:

  • An abstract class cannot be instantiated.
  • An abstract class may contain abstract methods and accessors.
  • It is not possible to modify an abstract class with the sealed (C# Reference) modifier because the two modifers have opposite meanings. The sealed modifier prevents a class from being inherited and the abstract modifier requires a class to be inherited.
  • A non-abstract class derived from an abstract class must include actual implementations of all inherited abstract methods and accessors.
2 Likes
165

The more I look at this class, the more confused I get. What does this:

   function popupReturn ($fieldarray, $return_from, $selection, $popup_offset=null)
    // process a selection returned from a popup screen.
    // $fieldarray contains the record data when the popup button was pressed.
    // $return_from identifies which popup screen was called.
    // $selection contains a string identifying what was selected in that popup screen.
    {

have to do with any of that database operation stuff? This is just one example but you have methods in there dealing with concepts well beyond the scope of database i/o. File uploads, “workflows”, “historic” and all sorts of GUI concepts like “buttons”, “popups” and “file pickers”

166

Sigh. If you don’t realize that isn’t a summary of what your class does then I’m afraid I need to leave this conversation now. I will leave you with a wikipedia article to read.

7 Likes
167

I always wondered whether that had a name.

168

I like the term “Monster Object”.

Scott

169

Let’s be clear about this. The sentence above is the pertinent sentence in that article to this whole discussion.

Tony, I’ve said this before in the other topics we’ve had the pleasure of discussing together. You are unfortunately way behind the times. I am really trying not to say this to be attacking. I want to you wake up! Think of it as me taking you by the shoulders and shaking you and screaming “Wake up Tony!” You are a passionate developer and the PHP world can always use people who have such passion. However, NOBODY today programs 9000 line monster objects. And trying to justify that DI is a bad things to use (even only in certain situations), based purely on your perspective and code, also shows clearly you have a “metacognitive inability as an unskilled developer to recognize your ineptitude”. It is a harsh reality to face. To wake up 10 years later and realize time has passed you by is disheartening at best and depressing at worse. To be honest, I’ve gone through that myself in a way. But yes, alas, the programming world, especially the PHP world, has changed. And actually, to be even more veracious, a 9000 line object had never been an accepted coding practice.

No Tony. You now have no leg of competency to stand on. You have no basis for showing you are worthy to listen to as a developer. And anyone that reads your posts and should even think they have any value, really should be aware, they are being led by a blind person, whose ideology is built on programming methodologies (and some selfmade one’s) made for the early 2000’s.

Work on your competency first. Either refactor your framework or start over. Learn the ways of today’s programming methodologies, then you can speak about how things might be wrong. And, if that is something you don’t want to do, then please accept that you are behind the times and just keep quiet and to yourself. Stay in your past. It will save you from looking like a fool. That is my heartfelt suggestion to you as a fellow developer. I really like your passion. I don’t like your ignorance and the arrogance that comes with your denial to change.

I too, am out of this discussion.

Scott

2 Likes
170

I agree 100% But tony has been in this position for the last 10+ years just look at his responses here to genuine and valid criticism: http://www.tonymarston.co.uk/php-mysql/your-code-is-crap.html#other.misconceptions

Let’s put it this way: Tony would rather spend time arguing pointlessly on the internet and updating his website to justify his badly written code, as highlighted by dozens of people here and elsewhere than reflect on the fact that the common denominator is himself.

Tony: If you’re going to discuss code quality, that’s fine, but please take Scott’s advice to heart. Instead of trying to justify your current codebase, start a meaningful discussion about how it can be improved. We’re willing to help, but you have to be open to the idea that your code is very much out of date a full of bad practices

What’s not really acceptable, and ultimately pointless is you coming here yelling to everyone “YER DOING IT WRONG” and refusing to engage in a discussion that isn’t just you trying to justify your poorly written code to yourself by seeking our approval of your misguided approaches.

1 Like
171

Then you obviously don’t understand the meaning of “encapsulation”, which is:

[quote]
The act of placing an entity’s data and the operations that perform on that data in the same class. The class then becomes the ‘capsule’ or container for the data and operations. [/quote]
This means that ALL the operations and ALL the properties go in the SAME class, so if an entity has 100 operations and 100 properties then they all go in the same class. The idea that a class should not contain more than N properties and methods, and each method should not have more than N lines of code is an artificial rule which I choose to ignore.

And before you accuse me of creating classes with too many responsibilities let me say that I have applied the Separation of Concerns and Single Responsibility Principle by separating out all database access into a separate component, and all UI logic into a separate component. If you don’t believe me take a look at http://www.tonymarston.net/php-mysql/infrastructure.html which describes my infrastructure in great detail.

172

If the rules of encapsulation produce a class with 9000 lines of code then nothing is wrong with that. On the contrary, if you split up a class in order to satisfy some arbitrary rules then you are guilty of breaking encapsulation and creating a system which is more difficult to maintain as the logic for an entity is now fragmented across multiple classes.

It is not just MY opinion that DI, when improperly used, creates more problems than it solves. Just tae a look at http://www.tonymarston.net/php-mysql/dependency-injection-is-evil.html#references which links to articles written by other developers.

If you accuse me of being ignorant simply because I refuse to follow your “advice” then I can only say “guilty as charged”. However, I do not place any value on your advice as I consider it to be nothing but dogma, and I am a pragmatist, not a dogmatist.

Good. Close the door behind you and take your dogmatism with you.

173

I started this discussion by asking two simple questions, and gave some practical examples of a situation where I thought that DI was not appropriate, but it is YOU and your cronies who refuse to engage in a sensible discussion. All you can do is tell me that my code is crap simply because I don’t follow the same methodologies, practices and techniques as you. There is no single way of coding, there is no single set of best practices. Each developer is allowed, or should be allowed, to develop software in his own unique way, and not be restricted by the short-sighted and dogmatic approach favoured by others.

I have said this before, and I’ll say it again. If I did everything the same as you then I would be no better than you, and I’m afraid that your best is simply not good enough. The first step to being better is to be different, and you have to admit that my approach is certainly “different”. It is sometimes called “thinking outside the box” or “stretching the envelope”. This is where progress is made. If everything stays the same then the result is stagnation, not progress.

You cannot say that my approach is wrong for the simple reason that it works, it works very well, and has been working for over a decade. Something that works cannot be wrong just as something that does not work cannot be right.

Here endeth the lesson. Don’t applaud, just throw money.

1 Like
174

So your approach, which is the exact approach that has been discussed in detail by developers from Google, IBM, Microsoft, Apple and Academics, authors and even those people who design the langues and deemed “bad practice” by all of them… is somehow superior to what exactly?

The exact concepts you’re trying to call “superior” have been demonstrated time and time again to be bad traits for software. Tight coupling, global state, action at a distance, These are all well documented. Please take Scott’s advice and actually learn something. You learnt to program 30 years ago and I’ll bet you’ve had no formal training since. Things change a lot in that time.

Try harder.

edit: I just wanted to reiterate Scott’s point as it hits the nail on the head:

You have zero credibility at this point. Nobody apart from you will agree that god class is an example of good design. That really is all there is to it.

1 Like
175

So by your reckoning, encapsulation means that you should make a single class that deals with:

  • buttons
  • file choosers
  • csv export
  • database
  • pdf file generation
  • version control
  • “workflows”
  • and various other irrelevant things.

By your definition encapsulation means that everything should go in one of 3 classes.

176

Hi. Hopefully I’m going to make the conversation a little messier. You’re both right and your both wrong… Honestly, I stop my reading on the 70th post.

On one side you have to practical @TomB, you’re failing here. There’s no business made of theory. Don’t get me wrong, I know what you mean by getting out the box and just rethink if the strategy is the appropriate.

@tony_marston404 , you need to get out of you shell, navigating on a 9000th line of code (or just opening in an editor will be a pain) and saying “it has to be” than you’re not thing it straight. Nothing “needs to be”. Is just like saying a sky scrapper is un-dividable. Well… in practice it is, but in fact you can think of floors, door, furniture, the structure (concrete, glass, iron… )

For both: nothing is right… nothing is wrong… it need to have a reason. A good practice by itself doesn’t mean anything if it doesn’t have a context. I won’t (probably) apply a SOLID principle to a 10 line batch to rename a file :). I won’t (probably) write a class with 200 methods for manipulating strings. Anything and everything makes sense couple but it can also make sense decoupled.

The questions are: Does it make sense? Have you though and discussed about it? Does it have stronger points that weaker points? Does it make sense to the business and is the business willing to pay for the investment of doing it right?

@TomB: the business doesn’t care how the software is made, it just wants the value of it and usually, they want it in a tight schedule and aren’t willing to negotiate or even understand your point of view, so the code needs it has to be dirty. There are scenarios, because of this, that DI can be evil because, there’s no question about it, it doesn’t make the drilling and navigation path fluid. You need to ask yourself “Which class implements this interface in this context”. And if you have legacy code that is highly couple… well trying to decouple it is a pain…
Also, trying to do DI for everything is not also practical, for every implementation you’ll need an interface, that almost “doubles” the work.

@tony_marston404, I regret that I didn’t make unit testing in some of my older programs because of business’s pressures, not good knowledge, just plain stupidity or laziness. Unfortunately some of the systems still live after 10 years and because I’ve lost the QA articles, because they weren’t written or because QA guy (that had that knowledge) left, I’m afraid of touching the code, and if I break… and it if does, can I detect it? Having quality unit tests pays of in the future (the unforeseen one), but this requires isolation and…. There’s no isolation if you can’t have DI.
Also, in my opinion you can’t pass a SQL query part to a business class and say it is decoupled… it isn’t, because … is part of an sql query, what if tomorrow you’re caching that entity or using a nosql or a redis backend ? Will it work?

Like I said, I stopped reading at the 70th post and don’t know if you really progress in the discussion (based on the last post you didn’t) so hopefully.

You’re both right… you’re both wrong. Use it wisely and with a reason.

3 Likes
179

That’s not really true. There’s no reason you need an interface, you can type hint a concrete class and inject that. In most cases that is sufficient as it allows subclasses and mocks.

If you’re using TDD this should very rarely, if ever, be an issue. Your test suite tests components in isolation so when you’re actually developing/debugging the code all you care about is the one class you’re testing. The implementation of dependencies will be mocks.

181

Right for the first part if you don’t mind of a module depend directly on another module that has the implementation, if you only want to depend on the contract then is not the case.
As for the second part, right again, if you’re on tdd. If you’re debugging a functional test, which you don’t have a test for it is easier to read a “if a the class_a else class_b” then to seek where the class comes from.

As you may see, my point is, it depends on the context. As long as you stick with a principle (could be better, could be worse) it will be fine. Nothing is perfect, that is a truth.

Now… for sticking with the idea of that 5000 loc class, with all those dependencies and responsibilities, is a good structure, you better have very strong arguments. Not saying that aren’t valid, but I wouldn’t put my hands on the fire for it. If you convince me, I will hire you. But that is another thread.

Anyway the original question was: Does it make sense you DI all the time? My answer is. Use it wisely.

182

The problem with this train of thought is that it ignores the fact that if you’re not using DI then you’re using a different approach for locating dependencies. All other methods have some pretty severe negative effects with flexibility, coupling and complexity. You must account for these in the equation when deciding which is “best”. I’ve yet to see a scenario where another approach is preferable to DI.

The negative effects of other approaches are magnified if you’re using TDD.

183

Thanks for the feedback. And the answer again is evasive… Sorry, it needs to be. And this article, which, for curiosity sake, I’m reading right now, explains it well.

Don’t try to do the marathon at the same speed you do the 100m. You won’t have the same stamina.

Sometimes you just have the time, or it can be overkill, or “don’t write stuff just because you have too”. Write it because you need it. Sometimes we can make it at steps. That is even the paradigm around tdd. Don’t write if you don’t have a purpose.

But I undestand what you mean. Most of the devs that I know that don’t do DI, do it because they don’t know how or are just too lazy… So must of the times are just excuses.

184

I’m not sure I understand this. In TDD you’d generally write the basics of the test before writing a single line of code that it relates to. You don’t write anything until you have to. In TDD it becomes more work to work around things like service locators and singletons than it is to just use DI.

Obviously for GUI components we can’t have automated tests, but the same principle applies. I can write a very basic script that loads a view/template file and pass it a mock model to test it essentially in the spirit of TDD.

185

@tomb you’re missing the point. You can shot one ant with a bazuka (it does implement the kill action) but is it really necessary? Isn’t the cost greater than just smashing it with your finger? Sometimes it just doesn’t pay off. You have to balance your decisions.