Dependency Injection: a discussion of the pros and cons

[s_molinari]

@tony_marston404

I object to is the argument that I MUST use DI in EVERY place where I have a dependency

I thought we all agreed it isn’t necessary all the time.

What we need to agree on is that DI is NOT evil or bad or anything in between, when used properly. It is a very good and useful design pattern for highly complex and long term applications. Because you don’t use it Tony, as prescribed or suggested or even given in numerous libraries, is also not any reason to say it is bad. Can’t you agree to that?

Scott

[TomB]

@tony_marston404

I’m afraid that this method of injecting dependencies just will not work in my framework because it changes the coupling between the Controller and the Model from very low to very high. My page controllers are pre-written and completely reusable by virtue of the fact that none of them contain any hard-coded names of the model classes which they instantiate. Each user transaction (unit of work) looks like the following:

This makes zero sense.

Instead of two files you can use one, which is this:

<?php
$table_id = "person";                      // identify the Model
require_once "classes/$table_id.class.inc";
$dbobject = new $table_id(new Whatever);
$result = $dbobject->doSomething();
?>

Which is both less code and skips an include, improving performance.

You haven’t even highlighted a problem with DI, you’ve highlighted an (easily resolved) design flaw in your own framework.

And I’m still waiting for an answer to this:

Dependency Injection: a discussion of the pros and cons

aha. So Tony, how is this: 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…

[TomB]

@fretburner

How is the above code any more tightly coupled than yours? The controller doesn’t know which model class will be passed in. Only the calling code knows which model and which controller classes are instantiated, which is no different from your example.

This is a good example of loose coupling. Tony’s singleton example has tight coupling: He can only ever use a model that wash hardcoded in the controller ($model will always be an instance of the same class), in this example, $model can be an instance of anything, making the controller work with any model.

[fretburner]

My understanding (and I’m sure Tony will correct me if I’m wrong) is that this code is acting as a page controller. It’s called directly by the end user:

person_enq.php

<?php
$table_id = "person";                      // identify the Model
$screen   = 'person.detail.screen.inc';    // identify the View
require 'std.enquire1.inc';                // activate the Controller
?>

The next piece of code Tony is referring to as the controller, but is essentially a module of resusable code, and it is dependent on $table_id and $screen being set in the current scope:

std.enquire1.inc

// Some other code before..

require_once "classes/$table_id.class.inc";
$dbobject = new $table_id;
$result = $dbobject->doSomething();

// Some other code after..

To be fair to Tony, if you combined the code into a single file as you’re suggesting then it does become hard-coded to one specific table, and unusable with others.

Note: for anyone who is interested, the full source that these examples come from is available on Tony’s site.

[TomB]

@fretburner

To be fair to Tony, if you combined the code into a single file as you’re suggesting then it does become hard-coded to one specific table, and unusable with others.

How? It’s already hardcoded by

<?php
$table_id = "person";                      // identify the Model
$screen   = 'person.detail.screen.inc';    // identify the View
require 'std.enquire1.inc';                // activate the Controller
?>

once $table_id is set in the code, it’s hardcoded and no different to having it in the same file.

edit: To clarify, I’m suggesting that the ‘std.enquire1.inc’ include file is not required:

$dbobject = new Person();
require_once 'person.detail.screen.inc'; 
$dbobject->doSomething();

p.s.

This has once again deviated into anecdotes, and specific implementations. The above has nothing to do with DI. Tony setting up his application in such a way that he can’t use DI is a problem with the application, not a problem with DI.

I am still waiting for minimal code examples that don’t use a specific implementation or require an understanding of anything other than the code presented in the argument. Although Tony has been drip feeding us excerpts from his application he has still failed to give us a complete, minimal working example that doesn’t have a dependency on his entire framework stack.

[tony_marston404]

@fretburner

$controller = new EnquireController(new Person); $controller->doStuff(); How is the above code any more tightly coupled than yours? The controller doesn't know which model class will be passed in. Only the calling code knows which model and which controller classes are instantiated, which is no different from your example.

Because you now have an outer object which instantiates the controller as well as activating its methods. I say “methods” in the plural as my controllers call several methods on the model object, then instantiate the view (will the model injected into it) so that it can format the data for output. In my implementation the first script simply passes control to the controller with a single require statement, after which the controller can do whatever it wants. Your idea of having a controller for the page controller is simply impractical.

You also fail to show how the controller injects any dependencies into the model.

[TomB]

@tony_marston404

Because you now have an outer object which instantiates the controller as well as activating its methods. I say “methods” in the plural as my controllers call several methods on the model object, then instantiate the view (will the model injected into it) so that it can format the data for output. In my implementation the first script simply passes control to the controller with a single require statement, after which the controller can do whatever it wants. Your idea of having a controller for the page controller is simply impractical.

You also fail to show how the controller injects any dependencies into the model.

Again Tony, please supply minimal working code examples that don’t have a dependency on your framework and include the bare minimum needed to make your point.

The way you’re using terminology to mean different things (e.g. singleton, controller, model) than anyone else is not helping you express your point in an understandable way.

[tony_marston404]

@s_molinari

tony_marston404:
I object to is the argument that I MUST use DI in EVERY place where I have a dependency

I thought we all agreed it isn’t necessary all the time.

Then why is it that people like TomB and others keep telling me that because I don’t use DI in every possible place that my code is crap?

@s_molinari

What we need to agree on is that DI is NOT evil or bad or anything in between, when used properly.

There are different definitions of what “properly” means, but you also omit the qualifier “where appropriate”. I DO use DI in certain places in my framework where it provides benefits that I can see such as:

• My transaction scripts inject the name of the model into the controller so that model names are never hard-coded into any controller. This makes each of my controllers reusable, and eliminates any tight coupling between the controller and the model.
• The controller injects the model object into the view so that it can extract the model’s data and format it for output. This makes each of my views (one for HTML one for PDF and another for CSV) reusable, and eliminates any tight coupling between the view and the model.

What I do NOT do is inject any dependencies that a model may have into the model when it is instantiated by the controller, instead those dependencies are hard-coded in the model and only instantiated when they are actually needed. I have been told that this makes my model classes less reusable but this is irrelevant as none of them is designed to be used outside of the application in which they were built. I do not build libraries of components so that developers can pick and choose which components they want, instead I build entire applications out of one or more subsystems (where each subsystem deals with a different database) which means that either the entire application is used or the entire application is not used. There is no halfway house.

I have been told repeatedly that I SHOULD use DI to deal with each model’s dependencies because it follows “best practice” and has obvious benefits, but I only see negative benefits in the form of more code to achieve the same thing in a more complicated way, and as a follower of the KISS principle I will always use the simplest solution that works. I have no need to use DI in my models, so I choose not to use DI in my models.

[TomB]

@tony_marston404

Then why is it that people like TomB and others keep telling me that because I don’t use DI in every possible place that my code is crap?

For at least the third time: DI adds flexibility, flexibility is an objectively desirable trait in software. You are arguing that DI is “evil” and/or pointless and gives no benefit which is a lie. I am not telling you your code is crap because you don’t use DI, I am saying your code would be objectively better if it was more flexible. However, this again is besides the point. You are arguing that DI is not the right tool for the job yet you have repeatedly failed to provide any code examples to demonstrate a better alternative despite being asked several times.

Edit: We do not care about your code at all. We care about programming concepts and best practices. You are arguing that DI has no benefit and keep citing your code as some sort of infallible biblical text which is the arbiter of all that’s good. We don’t care about your code or what it’s doing, which is why pointing to it as an example is equally pointless when discussing nuanced, abstract concepts.

You repeatedly argue from the perspective that your code is doing something therefore it must be doing it in the best way possible and everyone else is wrong to critique it. Until you start providing examples of concepts instead of your code all we can do is point at where your code can be improved. The only person to have brought up the topic of your code being “crap” is you. We’re having a discussion about the merits of DI and nothing else. The quality (or lack thereof) of your code has nothing to do with the topic at hand.

[fretburner]

@tony_marston404

I DO use DI in certain places in my framework where it provides benefits that I can see such as:

My transaction scripts inject the name of the model into the controller so that model names are never hard-coded into any controller. This makes each of my controllers reusable, and eliminates any tight coupling between the controller and the model.

The controller injects the model object into the view so that it can extract the model's data and format it for output. This makes each of my views (one for HTML one for PDF and another for CSV) reusable, and eliminates any tight coupling between the view and the model.

Let’s just be clear that you’re NOT using DI here, what you’re doing is setting and accessing variables in the global scope.

[tony_marston404]

@TomB

This makes zero sense.

Instead of two files you can use one, which is this:

<?php
$table_id = “person”; // identify the Model
require_once “classes/$table_id.class.inc”;
$dbobject = new $table_id(new Whatever);
$result = $dbobject->doSomething();
?>

Which is both less code and skips an include, improving performance.

You are missing the point AGAIN. Each user transaction in my application, and there are over 2,500, has a transaction script which looks like the following:

<?php
$table_id = "person";                      // identify the Model
$screen   = 'person.detail.screen.inc';    // identify the View
require 'std.enquire1.inc';                // activate the Controller
?>

Each time a new transaction is generated a new transaction script is generated, and it is also added to a menu so that it can be selected and processed immediately. Each transaction script specifies a pre-existing model, a pre-existing view and a pre-existing controller.

Each controller, of which there are only 40, is built into the framework and is never modified by the developer. No controller is ever tied to a particular model, and no model is ever tied to a particular controller. A controller can be used with any model, and a model can be used by any controller. With this mechanism I have achieved high reusability and low coupling which are supposed to be major objectives for using the OOP in the first place. I’m afraid that I am NOT going to throw away those levels of high reusability and low coupling just to follow your demands that I implement DI in a way that you approve. Your method would require that each of my 2,500 user transactions would have to have its own controller instead of being able to reuse one of the 40 existing controllers. All that effort but for what reward? Absolutely none, so it ain’t gonna happen.

[TomB]

@tony_marston404

Each time a new transaction is generated a new transaction script is generated, and it is also added to a menu so that it can be selected and processed immediately. Each transaction script specifies a pre-existing model, a pre-existing view and a pre-existing controller.

So you did not provide an example which adequately expressed your point and you’re blaming me for misunderstanding it.

All this stuff about models, controllers, menus, “transactions” whatever they are, etc has nothing to do with DI. Please just provide a minimal example of a situation where DI is the inferior solution.

Again: Please supply a minimal working code example that contains everything needed to express your point but nothing else. Until you do this, further discussion is pointless.

Edit: If you want to make your point clearly please provide two examples: One using DI and one not using DI so we can accurately ascertain that both solutions are doing the same thing.

[tony_marston404]

@TomB

fretburner:
How is the above code any more tightly coupled than yours? The controller doesn’t know which model class will be passed in. Only the calling code knows which model and which controller classes are instantiated, which is no different from your example.

This is a good example of loose coupling. Tony’s singleton example has tight coupling:

Have you noticed that I only use singletons when instantiating a dependent object in the model? Which means that for my controllers and views I have loose coupling, whereas tight coupling is restricted to each model’s dependencies. I use loose coupling where I can make those components reusable, but this is not necessary in the models as they cannot be used outside of the application in which they were built.

@TomB

He can only ever use a model that was hardcoded in the controller ($model will always be an instance of the same class),

Incorrect. None of my 40 reusable controllers has any model names hard-coded. The model names are passed down from one of the 2,500 unique transaction scripts. I only use singletons when instantiating dependent objects in the model where hard-coded dependency names do not cause an issue.

[TomB]

@tony_marston404

Incorrect. None of my 40 reusable controllers has any model names hard-coded. The model names are passed down from one of the 2,500 unique transaction scripts. I only use singletons when instantiating dependent objects in the model where hard-coded dependency names do not cause an issue.

how exactly is:

entrypoint.php

new Controller(new Model);

Less flexible than

entrypoint.php

$model_name = 'model';
requrire_once 'controller.php';

controller.php

$model = new $model_name;

//...

I can choose any controller and any model in the entry point in both examples.

The difference is, when using the first example I can also do this:

new Controller(new Model(new Address));

using the two-file method, I cannot choose the dependency in the “transaction script”

[tony_marston404]

@fretburner

My understanding (and I’m sure Tony will correct me if I’m wrong) is that this code is acting as a page controller. It’s called directly by the end user:

While both of these scripts could be called “controllers” it could cause confusion, so in my framework I use names like the following:

• person_enq.php is called a component script or transaction script as there is one of these scripts for each of the 2,500 user transactions in my application. When the user selects a menu option on the screen this is the script which is run.
• std.enquire1.inc is called a page controller (to differentiate it from a front controller) as there is one of these for each of the 40 or so transaction patterns in my framework.

It should be obvious that I could not obtain the same levels of reusability if I combined the 2,500 transaction scripts and 40 page controllers into single scripts.

[tony_marston404]

@TomB

fretburner:
To be fair to Tony, if you combined the code into a single file as you’re suggesting then it does become hard-coded to one specific table, and unusable with others.

How? It’s already hardcoded by

<?php
$table_id = “person”; // identify the Model
$screen = ‘person.detail.screen.inc’; // identify the View
require ‘std.enquire1.inc’; // activate the Controller
?>


You are missing the point. The script above is one of the 2,500 transaction scripts in my application whereas the require 'std.xxx.inc' line refers to one of the 40 page controllers. If you look carefully you will see that the transaction script actually “injects” the names of those dependent objects into the controller, in which case it is perfectly acceptable to have those names hard-coded in the component which does the injecting. This is what you have shown in your code samples, so it cannot possibly be wrong.

@TomB

To clarify, I’m suggesting that the ‘std.enquire1.inc’ include file is not required:

The 40 page controllers are totally separate from the 2,500 transaction scripts for a very good reason. They perform separate functions and always run in pairs. The page controllers can be reused by any number of transaction scripts. Each transaction script is unique as it refers to a user transaction within the application.

[TomB]

@tony_marston404

The 40 page controllers are totally separate from the 2,500 transaction scripts for a very good reason. They perform separate functions and always run in pairs. The page controllers can be reused by any number of transaction scripts. Each transaction script is unique as it refers to a user transaction within the application.

You didn’t explain that in the original post which is why I’ve been asking for complete examples. I have responded since. See my latest reply.

[TomB]

@TomB

new Controller(new Model(new Address));

To explain this in terms and code in your style. This achieves the level of flexibility that would be gained by doing this:

entrypoint.php

$table_name = 'person';
$dependency_name = 'address';
require_once 'controller.php';

controller.php

$table = new $table_name(new $dependency_name));

Hopefully you can appreciate that it’s more flexible.

What I don’t understand and I’d like an explanation of is why you cited this use-case as an example of where DI cannot be used.

[tony_marston404]

@TomB

Again Tony, please supply minimal working code examples that don’t have a dependency on your framework and include the bare minimum needed to make your point.

I cannot supply samples of code which do not have dependencies as every component in a layered application has dependencies.

• Each controller is dependent on a model, but the name of that dependency is injected from the transaction script.
• Each controller is dependent on a view, but the name of that dependency is injected from the transaction script.
• Each view is dependent on a model, that that object is injected into the view by the controller.
• Each model is dependent on a database object, but that is instantiated when required using a singleton where the name of the database and table are hard-coded with the class (remember that I have one class for each database table) and the name of the DBMS engine is supplied from the config file.
• If a model needs to communicate with any other objects they are instantiated when needed using a singleton.

@TomB

The way you’re using terminology to mean different things (e.g. singleton, controller, model) than anyone else is not helping you express your point in an understandable way.

Surely everybody reading this forum is familiar with the terms “model”, “view”, “controller”, “singleton” and “data access object”? I have not suddenly started to use these to mean something completely different, so where is the confusion?

[TomB]

@tony_marston404

Surely everybody reading this forum is familiar with the terms “model”, “view”, “controller”, “singleton” and “data access object”? I have not suddenly started to use these to mean something completely different, so where is the confusion?

“singleton” as described by books is a class which maintains its own single instance, you are using a static service-locator and calling it a “singleton”

“model” generally refers to a layer in the system not a specific class

“controller” is usually an object with methods rather than a single flat script.