Dependency Injection Breaks Encapsulation

Then you should learn to look at the structure diagram at http://www.tonymarston.net/php-mysql/infrastructure.html#figure5 before delving into the contents of a single class.

No, it is not a “God object”, it is an abstract table class which is inherited by every database table class. It contains code which may be used in any operation that may be performed on any database table.

As it is part of the framework it was not designed to be amendable by any application developer.

From: http://en.wikipedia.org/wiki/God_object

Most of such a program’s overall functionality is coded into a single “all-knowing” object, which maintains most of the information about the entire program and provides most of the methods for manipulating this data. Because this object holds so much data and requires so many methods, its role in the program becomes god-like (all-encompassing). Instead of program objects communicating amongst themselves directly, the other objects within the program rely on the god object for most of their information and interaction. Since the god object is tightly coupled to (referenced by) so much of the other code,

From: http://en.wikipedia.org/wiki/God_object

Since the god object is tightly coupled to (referenced by) so much of the other code, maintenance becomes more difficult than it would in a more evenly divided programming design.

From: http://misko.hevery.com/code-reviewers-guide/flaw-class-does-too-much/

“You can look at anything except for this one class”

So yes, it fits the definition 100%.

I suggest you read what I wrote at the start of this discussion:

All I have done is provide examples showing where DI does not provide any benefits, and all you can keep saying is “but you should be using DI anyway”.

No, I shouldn’t need to read the documentation to determine if your “Default Table” class is dealing with database or HTML tables. It should be able to meet that baseline of clarity from naming conventions and comments alone - that is it should be able to self document itself at least to that standard. If it can’t something is wrong

Can you even tell me all of what that class does in 20 words or less? Just from glancing at the declared variables I doubt it. If you can’t give a concise description of what the class does then the class is itself inconcise and should be broken apart into two (or likely more) related classes.

I’m not trying to be mean here, but I’ve seen old code bases that have been home grown over a decade without ever being pruned and been paid to work on them. I don’t think you’re stupid, but I don’t think you’re any smarter than the guys who wrote those programs either and they where replete with gotcha’s and minefields typical of the ball of mud pattern that made doing anything more than slightly outside the program’s main purpose a nightmare. And if having, what, 50+ public variables on one “default” class object isn’t a ball of mud then nothing is.

3 Likes

It is an abstract class that contains methods for any operation that can be performed on any database table. [that’s 19 words]

That is not a “God object” by virtue of the fact that it does not do “everything” in a program, it is merely the Model class in the Model-View-Controller design pattern. If it combined all the View and Controller code in the same class then, and only then, could it be described as a “God object”.

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?

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

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”

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

I always wondered whether that had a name.

I like the term “Monster Object”.

Scott

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

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

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.

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.

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

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