Dependency Injection Breaks Encapsulation

This is demonstrably false yet again and I have already done this several times… but fine… how is:

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

class singleton
// ensure that only a single instance exists for each class.
{
    function &getInstance ($class, $arg1=null)
    // implements the 'singleton' design pattern.
    {
        static $instances = array();  // array of instance names

        if (array_key_exists($class, $instances)) {
            // instance exists in array, so use it
            $instance =& $instances[$class];
            
        } else {
            // load the class file (if not already loaded)
            if (!class_exists($class)) {
                switch ($class) {
                    case 'date_class':
                        require_once 'std.datevalidation.class.inc';
                        break;

                    case 'encryption_class':
                        require_once 'std.encryption.class.inc';
                        break;

                    case 'validation_class':
                        require_once 'std.validation.class.inc';
                        break;

                    default:
                        require_once "classes/$class.class.inc";
                        break;
                } // switch
            } // if

            // instance does not exist, so create it
            $instances[$class] = new $class($arg1);
            $instance =& $instances[$class];
        } // if

        return $instance;

    } // getInstance
    
} // singleton

 new Person();

Fewer lines of code than:

class Person {
    public function __construct(Address $address) {
        $this->address = $address;
    }

    function doStuff () {
        ....
        $address = $this->address->getPrimaryAddress($customer_id);
        ....
    } // doStuff
} // end class Person

$address = new Address;
new Person($address);

You can actually download Tony’s framework. I did out of interest. This is used on every single page for everything as far as I can tell. Judge for yourself: http://pastebin.com/m76ZAUZc This is from the guy who claims to have a “Minimalist approach”: http://www.tonymarston.net/php-mysql/minimalist-approach-to-oop-with-php.html and “Keep it simple”.

1 Like

I suggest you read post #90 where TomB says the following

[quote]
Even in situations where DI doesn’t directly add a benefit it’s still preferable any of the other approaches you have listed.[/quote]

Incorrect. The reason for not using DI is that I have no need to inject alternative dependencies simply because there are no alternatives. Using DI in the wrong place does not automatically make your code “better”, it can actually make your code worse.

If you are saying that modern OOP requires that I use DI even in those places where it is not appropriate, then I have to disagree. OOP does not require me to use any particular set of design patterns. I will use a particular design pattern when it is appropriate to do so, and when it is not appropriate then I won’t.

They are if they implement a design pattern without thinking if it is the right solution for the problem at hand.

If they cannot evaluate DI, or any other design pattern for that matter, to see if it is the right solution for the current situation then every solution which they implement is liable to be the wrong solution. It takes brain power to implement the right solution. Any idiot can implement an inappropriate solution.

I read the variable list (which is longer than many of my class files if you prune the comments) and I couldn’t determine if the class was dealing with database tables, html tables or some type of PDF distillery. If a ten year veteran programmer can’t make that determination by line 100 the code is not straightforward. If anything it’s a swiss army knife class.

1 Like

The common name is “God Object”: http://en.wikipedia.org/wiki/God_object But yes, it’s incredibly unclear what it does, making any changes to it would be… problematic as it’s difficult to know what knock-on-effects it will have and there are no test cases remember!

1 Like

That in turn is part of this tried and true anti-pattern - Ye 'Ole Big Ball of Mud.

1 Like

I worked alone on my framework, although some other developers supplied some bits and pieces which I found useful. Other developers have used my framework to write their own applications. I developed my main ERP application on my own, but I am now sharing it with my business partner who has added some modules of his own. He has even trained up some developers in India to use my framework. Both he and his Indian developers like it because it is not filled with useless rubbish, is easy to understand, enables new components to be developed very rapidly, and has features which allow any sort of customisation to be quickly and easily added for any customer.

It is easy to memorise the file paths for my classes as there are only two possible locations:

  • The INCLUDES folder supplied with the framework
  • The CLASSES folder within each subsystem.

I don’t have complex class hierarchies or complex directory structures, so finding the right class file is as easy as falling off a log.

That is why I have a business partner who helps sell the ERP application which I wrote, and which we have enhanced together with the aid of his Indian developers. Our joint efforts have recently resulted in a significant sale to a major multi-national corporation. If it was such “unmaintainable crap” as some of the posters to this discussion have intimated, then none of this would be possible.

Incorrect. Tens of thousands, sometimes hundreds of thousands.

Remember that I wrote both my framework and my application years before autoloaders existed, and there would not be any benefit in changing my code now.

This simply isn’t true either. Your own code aside, CEOs and key decision makers in large organisations’ priorities are often different to the technical part of the company. They look at the bottom line. It’s difficult to explain the concept of code quality to these people simply because they don’t have the background to understand it. They look at the cost and little else. PHP vs ASP.NET? Which is cheaper? Go with that. Business people are not interested in the same things programmers are and while it can be easy to demonstrate the difference between Notepad and Word, it’s difficult to explain the difference between tightly coupled and loosely coupled code without taking them on a long, expensive course… for little benefit to them.

Oh my. :open_mouth:

Scott

And remember that this discussion would be better if it were conceptual rather than discussing existing implementations. Nobody is asking you to change anything, we’re just using your code to highlight bad practices and as an example of where improvements can be made.

[quote=“tony_marston404, post:149, topic:113596”]
Other developers have used my framework to write their own applications.[/quote]

What does that prove? People use WordPress for crying out loud, and a more convoluted demonic untestable piece of trash has yet to be equaled nor would I want to see it.

[quote=“tony_marston404, post:149, topic:113596”]
I developed my main ERP application on my own, but I am now sharing it with my business partner who has added some modules of his own. He has even trained up some developers in India to use my framework. Both he and his Indian developers like it because it is not filled with useless rubbish, is easy to understand, enables new components to be developed very rapidly, and has features which allow any sort of customisation to be quickly and easily added for any customer. [/quote]

I’m not convinced, not after reading the variable list attached to that one class.

That’s still two more locations than I have to memorize.

Not as easy as not having to do it though.

Hundreds of thousands is still south of a million my friend. And remember, that’s the smallest contract we’ve signed in three years.

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