10 Replies - 959 Views - Last Post: 15 May 2016 - 05:00 PM

#1 e_i_pi  Icon User is offline

  • = -1
  • member icon

Reputation: 879
  • View blog
  • Posts: 1,893
  • Joined: 30-January 09

DI Containers break the S in SOLID?

Posted 12 May 2016 - 08:54 PM

Woohoo, super boring thread about OOP, SOLID and DI! Let the stalwart opinions loose! But just before we get to that,. here's a picture of a cute puppy to relieve the stress that is sure to abound...


Posted Image



So, I code in PHP (*dodges rotten fruit and shoes*) and I'm developing an application with IoC and DI. The problem I have is that there is a lot of this going on:
Spoiler

What a nightmare. When you consider that the DI of these controllers and private variables is replicated across multiple front controllers, models, other controllers, handlers, etc. I end up spending an awful amount of time doing this plumbing instead of doing work.

So I think to myself "what is a nice solution here?" and I start looking around at how other applications handle this, and the term DIC (Dependency Injection Container) crops up a lot. The basic idea of a DIC is this:
  • You hate doing the code I do above
  • You create a DI Container, so that instead of passing in multiple objects via DI, you pass in one object, the DIC
  • The container code is hard, so you have a cry, wipe away the tears, and download some rubbish that a 3 year old has written
  • When you go to use the DIC and retrieve a controller, you call it like this: $this->_container->get('magicstring')

For those of you who don't know what a magic string is, look at this article in Wikipedia which talks about magic numbers, that's what I'm talking about. Basically, a string value that comes from nowhere and has some arbitrary meaning, which in turn makes refactoring a god awful nightmare.

Obviously (from my tone) the standard implementation of a DIC smells like dog leavings. So I'm thinking a better approach would be to have a DI Container object that looks like this:
class MyContainer extends IContainer
{
	private
		$_studentController
	;
	
	public function setStudentController(IStudentController $studentController = null) {
		$this->_studentController = $studentController ?: StudentController::getInstance();
		return $this
	}
	
	public function getStudentController() {
		if(is_null($this->_studentController) {
			$this->setStudentController();
		}
		return $this->_studentController();
	}
}


For those of you who can't read PHP, no worries, let me explain it.
  • The DI Container is an object that gets instantiated somehow (I would think Singleton pattern, which yes is an anti-pattern in persistent applications, but PHP is different, so don't get all hot and bothered) and passed into the front controller / model / handler / whatever
  • The front controller calls $this->container->getStudentController()->someFunction()
  • As such, because it's not using a magic string, you can use code inspection / your IDE / whatever to figure out that the StudentController is used by that particular front controller

To me, this seems like a neat solution, and replaces the nightmare of configuring a DI Container with a call-on-demand type pattern that responds well to refactoring, code completion, and code sniffing.

The problem is, using a DI Container in any way whatsoever is a violation of the holy scriptures of SOLID, in particular, the S part - Single Responsibility Principle. I have read a lot of blogs / posts / etc decrying the evils of DI Containers, but like most high-end philosophical debates about OOP and programming, it seems to just be regurgitated crap from some nabob who gave a talk in 2003, and everyone just repeats what he/she said in the hope that they too look important and knowledgeable. I have yet to find an alternative to DI Containers that is SRP, whatever that means. Apparently you just don't DI Container while SRPing. It's that simple.

How the pattern I have described above (in code) looks to me like a service locator of sorts, another anti-pattern apparently. I'm having trouble wondering what on earth it is you're meant to do to be compliant with the Holy Trinity of SOLID. I'm not doggedly going after some reverence bestowed by Robert C Martin, but I do want to try to understand what the SOLID solution is to this nightmare of DI that results in a lot of violations of DRY. It's starting to feel a little like this, can anyone shed some light please...
Posted Image

Is This A Good Question/Topic? 1
  • +

Replies To: DI Containers break the S in SOLID?

#2 astonecipher  Icon User is offline

  • Senior Systems Engineer
  • member icon

Reputation: 2517
  • View blog
  • Posts: 10,093
  • Joined: 03-December 12

Re: DI Containers break the S in SOLID?

Posted 12 May 2016 - 09:17 PM

Judging from the variable names, you are passing in Interfaces for the constructor?
Was This Post Helpful? 0
  • +
  • -

#3 e_i_pi  Icon User is offline

  • = -1
  • member icon

Reputation: 879
  • View blog
  • Posts: 1,893
  • Joined: 30-January 09

Re: DI Containers break the S in SOLID?

Posted 12 May 2016 - 09:19 PM

Yeah, take them to mean interfaces. In the actual code, I'm not using interfaces, just base classes, but let's assume for arguments sake we pass in interfaces, like you're meant to with Dependency Injection.

This post has been edited by e_i_pi: 12 May 2016 - 09:20 PM

Was This Post Helpful? 0
  • +
  • -

#4 astonecipher  Icon User is offline

  • Senior Systems Engineer
  • member icon

Reputation: 2517
  • View blog
  • Posts: 10,093
  • Joined: 03-December 12

Re: DI Containers break the S in SOLID?

Posted 12 May 2016 - 09:35 PM

I'm all for dependency injection, but rather than crowd the constructor, I would only pass the base classes I need in those methods.
Was This Post Helpful? 0
  • +
  • -

#5 e_i_pi  Icon User is offline

  • = -1
  • member icon

Reputation: 879
  • View blog
  • Posts: 1,893
  • Joined: 30-January 09

Re: DI Containers break the S in SOLID?

Posted 12 May 2016 - 09:52 PM

They are the base classes I need, that's the problem. Any particular front controller (or ordinary controller for that matter) might require access to 3-6 other controllers. Think of this relational structure:
  [Student]                        [Partner]
       |                               |
    [Student Application]-----\    [Position]---\
                    |          \-----+-------[Cycle]
                  [Shortlisted Applicant]-------/
                      |              |
               [Interview]        [Placement]


It's a mess to look at, sorry, but fixing the Visio licencing issue is on the backburner at the moment while I put out other fires.

This post has been edited by e_i_pi: 12 May 2016 - 09:55 PM

Was This Post Helpful? 0
  • +
  • -

#6 astonecipher  Icon User is offline

  • Senior Systems Engineer
  • member icon

Reputation: 2517
  • View blog
  • Posts: 10,093
  • Joined: 03-December 12

Re: DI Containers break the S in SOLID?

Posted 12 May 2016 - 10:01 PM

The controller is needed or the base class is needed?

This is what I mean, when I say the method can grab what it needs:
public function topCharts() {
		$bhUser = new \BH\user($this->db, $this->userID); 
		  
		$this->agencyID = $bhUser->getAgencyID();
		$this->vars["agencyID"] = $this->agencyID;
	
		$campaign = new \BH\campaign($this->db, $this->agencyID, $this->userID);		
	    $worksheet = new \BH\worksheet($this->db, $this->agencyID, $this->userID);		
		$fromDate = date("Y-m-d",strtotime());
		$campaignCounts = $campaign->countCampaignsByMonth($fromDate,15);
		$buysCounts = $worksheet->countBuysByMonth($fromDate,15);
		$adsCounts = $worksheet->countLinesByMonth($fromDate,15);
		

Was This Post Helpful? 0
  • +
  • -

#7 e_i_pi  Icon User is offline

  • = -1
  • member icon

Reputation: 879
  • View blog
  • Posts: 1,893
  • Joined: 30-January 09

Re: DI Containers break the S in SOLID?

Posted 12 May 2016 - 10:31 PM

The controller is needed. Perhaps I've got my MVC pattern muddled up, but I use the Controllers as kind of a traffic warden, servicing requests and acting as the middle man between the DAL and the calling controller (usually a front controller, which itself has been accessed via a page request).

The way my application is set up is like this:
  • User accesses a page via http://mydomain/application/page.php
  • page.php creates a MasterPage object, Dependency Injecting the relevant front controller, e.g.:
    echo (new \InternshipAdmin\Page\Master\MasterPage(
        (new \InternshipAdmin\FrontController\StudentManageInterviewsPage())
    ));
    
    

  • The front controller constructor has all the DI controllers, and defaults them to the ordinary (non-mock stubbed) controllers:
    final public function __construct(
    	UserController                      $userController                     = null,
    	PartnerPositionController           $partnerPositionController          = null,
    	CycleController                     $cycleController                    = null,
    	StudentApplicationController        $studentApplicationController       = null,
    	InterviewController                 $interviewController                = null
    ) {
    	$this->_userController              = $userController                   ?: UserController::getInstance();
    	$this->_partnerPositionController   = $partnerPositionController        ?: PartnerPositionController::getInstance();
    	$this->_cycleController             = $cycleController                  ?: CycleController::getInstance();
    	$this->_studentApplicationController= $studentApplicationController     ?: StudentApplicationController::getInstance();
    	$this->_interviewController         = $interviewController              ?: InterviewController::getInstance();
    	$this->_processRequest();
    }
    
    

  • In that constructor there is a processRequest() method, which looks like this:
    final private function _processRequest() {
    	// Non-students redirect
    	if(!$this->_userController->isUserStudent()) {
    		$this->_redirectToUsersHomePage();
    	}
    	
    	// Set default values
    	$this->_action = isset($_REQUEST[FormParameterConstants::ACTION])
    		? $_REQUEST[FormParameterConstants::ACTION]
    		: FormValueConstants::NO_ACTION;
    	$this->_id = isset($_REQUEST[FormParameterConstants::ID])
    		? $_REQUEST[FormParameterConstants::ID]
    		: null;
    	$this->_selectedCycleID = isset($_REQUEST[FormParameterConstants::CYCLE_ID])
    		? $_REQUEST[FormParameterConstants::CYCLE_ID]
    		: null;
    	if(is_null($_REQUEST[FormParameterConstants::CYCLE_ID])) {
    		$this->_redirectToUsersHomePage();
    	}
    	
    	switch($this->_action) {
    		case FormValueConstants::ACCEPT:
    			$interviewModel = new InterviewModel($this->_id);
    			$interviewModel->setIsStudentAttending(true);
    			$this->_interviewController->save($interviewModel);
    			break;
    		case FormValueConstants::REJECT:
    			$interviewModel = new InterviewModel($this->_id);
    			$interviewModel->setIsStudentAttending(false);
    			$this->_interviewController->save($interviewModel);
    			break;
    		case FormValueConstants::NO_ACTION:
    			break;
    		default:
    			throw new Exception('Invalid action given in call to ' . __METHOD__. '.  Action given was "' . $_REQUEST[FormParameterConstants::ACTION] . '".');
    	}
    	$this->_viewModel = new ViewModel(
    		$this->_interviewController->getInterviewModelCollection_ByStudentApplication(
    			$this->_studentApplicationController->getExistingStudentApplicationID(
    				$this->_userController->getUserModel()->getUsername(),
    				new CycleModel($this->_selectedCycleID)
    			)
    		)
    	);
    }
    
    

You can see in the switch-case block the controllers being used to save the models. You can also see the controllers being used later on to DI the ViewModel with an InterviewModelCollection (basically, a class that spits out an Iterable set of InterviewModels).

So here, the controllers are acting as a go-between for retrieving and saving models to the data layer. The models themselves are awfully dumb, and know nothing of the DALs let alone the database. The controllers know about the DALs, and the DALs know about the DBAccessor, which in turn knows about the database. The front controller only knows about the controllers, and tells them to do any model saving/loading - the front controller simply handles the request and prepares the ViewModel (and View - code not shown) so that the output can be spat out to the user.

Maybe my design is wrong, I don't know, it's what I'm going with. In any case, I'm tired of "bootstrapping" front controllers, controllers, models, and everything else with some arbitrary set of controllers, when instead I could use a DI Container. BUT, DI Containers are the devil, allegedly, and I haven't seen any real argument against them other than "it violates the 1st commandment", or any alternate solution that makes sense.

This post has been edited by e_i_pi: 12 May 2016 - 10:32 PM

Was This Post Helpful? 0
  • +
  • -

#8 astonecipher  Icon User is offline

  • Senior Systems Engineer
  • member icon

Reputation: 2517
  • View blog
  • Posts: 10,093
  • Joined: 03-December 12

Re: DI Containers break the S in SOLID?

Posted 12 May 2016 - 10:45 PM

I know all about working with what you are dealt or what is already inplace, I don't fault you for that. I would say the implementation is muddled.

The snippet I showed, each controller controls its own sub. So, in your case, one controller calls another to do anything. In mine, each controller is accessible and inherits from the base controller. That controller can call its' database model for data and render the views.

 $this->_interviewController->save($interviewModel);

This would be sitting in my DAL, not my controller.

The DIC may likely make your life easier. Time will tell if it was a wise choice one way or another, but who doesn't regret implementation decisions eventually?
Was This Post Helpful? 0
  • +
  • -

#9 e_i_pi  Icon User is offline

  • = -1
  • member icon

Reputation: 879
  • View blog
  • Posts: 1,893
  • Joined: 30-January 09

Re: DI Containers break the S in SOLID?

Posted 12 May 2016 - 11:34 PM

Quote

I know all about working with what you are dealt or what is already inplace, I don't fault you for that. I would say the implementation is muddled.

It's not working with what I'm dealt with. I was dealt with a non-OOP large scale application that I am systematically pulling apart and re-coding piece by piece. The design is my own based on my experience with MVC and OOP, which is still in a developing state unfortunately. My understanding is/was this:
  • View - Basically just a template or skeleton that is utterly stupid, and receives a ViewModel (or set of Models) that it references in order to replace placeholders or inject values, before being ready for display
  • Model - A reasonably dumb object that has:
    Attributes that align with database columns
    Getters and setters that align with those attributes
    Getters that deliver non-stored derived attributes (e.g. isAlreadyProcessed(), hasGreatEyebrows(), countOfBananaPeels())
    Relational getters that reference child/parent models (e.g. $city->getStateModel()->getCountryModel())
  • Controller - A traffic cop that is responsible for handling specific requests and ensuring calls made to it aren't stupid, and calls it makes are standardised. Controllers could be:
    Front controller - Handles a page request, ensuring get/post variables aren't stupid
    Domain model controller - Handles requests relevant to a particular domain model, ensuring IDs/models/etc are typed correctly
    Handler - A service-oriented controller which handles requests for a particular service, such as a MailHandler, SessionHandler, etc

On top of that, we have certain other object types which don't fit neatly into the MVC design, such as:
  • DALs - Objects responsible for saving and retrieving data to a data source
  • Configurators - Objects that handle configuration of components, such as SMTP, Database, File System
  • Type Helpers - Bad name, but basically something that gets a typed variable as an input, which can then be spat out in a format, or have something special done with it. An ArrayHelper might have methods that sort the array on certain conditions for example
  • Constants - Abstract classes that store specific constants. They might mirror static lookup values in a database table, or be similar to an ENUM setup


Maybe I've got it wrong, as I said I still feel like I'm learning at times, even though I've been doing this for years.

Quote

This would be sitting in my DAL, not my controller.

The save() method in the controller calls the save() method in the DAL, it simply passes the object through. It's things like the delete() method that necessitate the controller intervening, as it "cleans" the input variable, see below:
// Controller methods
final public function delete($interview) {
    return $this->_interviewDAL->delete($this->_resolveInterviewID($interview));
}
    
final private function _resolveInterviewID($interview) {
    if(is_numeric($interview)) {
        return $interview;
    } else if(is_object($interview) && (get_class($interview) === get_class(new InterviewModel()))) {
        /** @var $interview InterviewModel */
        return $interview->getID();
    } else {
        return null;
    }
}


That's a naive example above, there are more complex methods that have three input variables, each of which can come in ID or Object format. I use the controller to standardise the calls to the DAL, so that the DAL methods don't have to think too much about the data coming in:
// DAL method
function delete($interviewID) {
  $query = 'DELETE FROM `interview` WHERE `id` = :id';
  $boundValues = [':id' => $interviewID];
  $parameterSet = (new DatabaseParameterSet())->setParameters($query, $boundValues);
  return $this->dal->execute($parameterSet);
}


Was This Post Helpful? 0
  • +
  • -

#10 Skydiver  Icon User is online

  • Code herder
  • member icon

Reputation: 6164
  • View blog
  • Posts: 21,252
  • Joined: 05-May 12

Re: DI Containers break the S in SOLID?

Posted 13 May 2016 - 05:43 AM

Check out what D stands for in SOLID.

The reason why your particular implementation of a DI container is breaking the single responsibility principle is because you have the classes hardcoded into it. A container, in the general sense of the word in computer science, is meant to support generic data - ala list, tree, heap, etc. Alas, you are using a language where you have limited choices, as well as made a design decision to not allow the use of magic strings in the context of a service locator.

The other thing that is causing you to have a lot of work is that you are setting up your classes such that if you don't pass in its dependencies, you are having it go get it itself. Don't do that. One of the points (at least to me) of inversion of control is that you make it dead obvious what a class depends on. If I fail to pass in the correct parameters, the compiler tells me that I'm stupid.

I can sort of see though why you are defaulting the constructor parameters: Now you have a ton of typing to do whenever you need to instantiate any given class because of its dependencies. So either pay the price once while writing the constructor, or pay many times each time a class needs to be instantiate. May I recommend a factory like pattern?

This post has been edited by Skydiver: 13 May 2016 - 10:42 AM
Reason for edit:: Added link to SOLID; added other OP constraint about no magic strings.

Was This Post Helpful? 0
  • +
  • -

#11 e_i_pi  Icon User is offline

  • = -1
  • member icon

Reputation: 879
  • View blog
  • Posts: 1,893
  • Joined: 30-January 09

Re: DI Containers break the S in SOLID?

Posted 15 May 2016 - 05:00 PM

View PostSkydiver, on 13 May 2016 - 11:43 PM, said:

The reason why your particular implementation of a DI container is breaking the single responsibility principle is because you have the classes hardcoded into it. A container, in the general sense of the word in computer science, is meant to support generic data - ala list, tree, heap, etc. Alas, you are using a language where you have limited choices, as well as made a design decision to not allow the use of magic strings in the context of a service locator.

I guess this is where I'm confused about SRP, and where I perceive that it gets more philosophical/religious, and less hard and fast facts.

The pattern I'm describing (I believe) is a type of service locator (thought I could be wrong). Surely then, then single responsibility of this class is to act as a request router, kind of like a freeway interchange. The benefits I see of having a hard-coded service locator are:
  • Assigning default controllers / classes / etc (which is 99% of your work) is handled automatically, cutting down the amount of time you're hammering out plumbing code
  • Reduces the DI footprint on constructors
  • The service locator could be reconfigured on the fly, allowing for creating different mock stub profiles for each unit test you want to do (caveat: I haven't even started unit testing, I am trying to make sure that my code is conducive to it)

So maybe calling it a container or service locator is semantically incorrect. What I'm trying to achieve is:
  • Cuts down on plumbing code
  • Retains IntelliJ/IntelliSense (i.e. cannot use magic strings)
  • Can facilitate unit testing, or at least not prevent it
  • Complies with SOLID if possible


Quote

Check out what D stands for in SOLID.

The other thing that is causing you to have a lot of work is that you are setting up your classes such that if you don't pass in its dependencies, you are having it go get it itself. Don't do that. One of the points (at least to me) of inversion of control is that you make it dead obvious what a class depends on. If I fail to pass in the correct parameters, the compiler tells me that I'm stupid.

I have looked at the D in SOLID, and thought I was achieving that by having the dependencies in the constructor, rather than defined ad-hoc inside the constructor / methods?

It looks to me like what you're describing in moving the responsibility of injected class instantiation all the way to the top of the call tree which, when I looked at this initially, seemed grossly counter-intuitive. Consider the following:
  • Class A has Method B that relies on Controller C, so we have to pass Controller C in when calling Method B
  • Controller C relies on Controllers E and F
  • Controller E relies con Controllers G, H, and I
  • Controller F relies on Controllers J, K, and L

If we then want to call Method B, we need to (at some point) pass in Controllers C, D, E, F, G, H, I, J, K, and L. This seems to be a nightmare scenario to me, not in the amount of typing that needs to be done (though that is true), but what happens when you start to refactor and/or partially replace objects?

Quote

I can sort of see though why you are defaulting the constructor parameters: Now you have a ton of typing to do whenever you need to instantiate any given class because of its dependencies. So either pay the price once while writing the constructor, or pay many times each time a class needs to be instantiate. May I recommend a factory like pattern?

My understanding of a Factory Pattern is that it abstracts away the construction of the class with all dependencies resolved. Isn't that what I'm doing with the code in the OP by calling getStudentController()? It seems the main difference between the Factory Pattern and my design is that with the Factory Pattern you extend the base class and implement the abstract methods, whereas in my pattern you would use the base class and instead pass configurations.
Was This Post Helpful? 0
  • +
  • -

Page 1 of 1