2

I'm moving from Globals and Singletons (=bad?) to Dependency Injections (=good?) in PHP and I'm very new to it.

I have read many related topics on Stack Overflow already but I still can't understand the main principles of DI.

Please tell me if I'm doing it right (it's just shortened pseudo-code):

// first connect to DB
$sql = new Sql(); 

// create logger (I'm writing logs to Database so I need to pass $sql to it)
$log = new Log($sql);

// restore logged in user, get info about user, unread messages... 
// class "User" needs  access to Database and Logs:
$user = new User($sql, $log); 

// now we need to collect all the data of current section of my Website
// I'm using Model and Controller:
$model = new FrontPageModel($sql, $user, $log); 

$pageController = new FrontPageController($model);

Though it may look OK at this step but what if I need to get access to more classes like Config, Session, etc.?

Should my code transform to this?

$model = new FrontPageModel($sql, $user, $log, $config, $session); 

Isn't it too much already?

I know someone could advice to use a kind of big "Application" class and put Config, Session, Log, Db objects inside of this class but I feel that it isn't very good idea.

Next question - what if I need to get logged User's ID inside of my FrontPageController? I did not pass an instance of "User" to FrontPageController, but it was passed before (in chain) to FronPageModel.

class FrontPageController{
  private $model;
  function __construct($model){
    $this->model = $model;
  }

  function getData(){
    echo $this->model->user->id; // is it right way?
  }
}

That "$this->model->user->id" seems like overkill to me.

Roman
  • 3,799
  • 4
  • 30
  • 41
  • Related: http://stackoverflow.com/questions/4603555/how-to-deal-with-constructor-over-injection-in-net/4603666#4603666 – Mark Seemann Jan 12 '12 at 16:05

3 Answers3

4

It might not be the prettiest, but you're certainly not doing it "wrong". You're demonstrating classing constructor injection, but you could perhaps refactor to keep some of the disparate objects separated a bit more.

My suggestion would be to look at established PHP DI containers. See how they work, use them in a few apps, and (1) you'll have much more testable apps and (2) you'll be much more comfortable with DI in general.

Take a look at one or more of the following:

Jeremy Kendall
  • 2,869
  • 17
  • 17
  • 1
    I read Symphony DI Documentation and it looks not better than just another universal global "Registry" pattern. We're coming back from where we came. Tell me if I'm wrong. What's the difference? – Roman Jan 13 '12 at 07:22
  • 1
    I can't explain it anywhere near as well as Martin Fowler, so I'll let him do it ;-) http://martinfowler.com/articles/injection.html – Jeremy Kendall Jan 14 '12 at 05:07
3

Like all the joyful stuff, it' depends.

I would put everything inside a factory class, or several factory classes for different sub systems.

class AppFactory {
    protected $_sql;

    public function getSql()
    {
        if (!empty($this->_sql))
            return $this->_sql;

        $this->_sql = new Sql();
    }
}

Then initial it on application run. Configure if needed.

$factory = new AppFactory();
// set config 
// $factory->internallyGenerateSingletons = true;

And now every other class can use it. And of course you have injection to put factory inside.

$model = new FrontPageModel($factory);
// or
// $model->useFactory($factory);

Regarding other question. Is this the right way.

...
$this->model->user->id
...

I would say it's ok but batter is to use methods like this:

...
$this->model->getUser()->id
..

So you can mock user class when doing unit testing. And it makes application more flexible.

Matej Baćo
  • 1,312
  • 2
  • 10
  • 12
  • so when I'm inside of FrontPageModel and I need access to Database, I should call $this->factory->getSql()->getRow('select id, name from items')? – Roman Jan 13 '12 at 06:38
  • Yea, if you want to :) If line is too long you can store parts inside variables, get more shorter lines of coder. – Matej Baćo Jan 13 '12 at 09:40
  • 1
    Some people prefer not using variables, having all in one line. I myself will split it. `$this->factory->getSql()->getRow('select id, name from items')` is too long. I would prefer `$sql = $this->factory->getSql(); $row = $sql->getRow('select id, name from items');` – Matej Baćo Jan 13 '12 at 09:44
2

It seems like you have got the idea of dependency injection right. You even question Symfonies idea of DI (in a comment), which shows more mature understanding.

A next step to solving the questions you have is to take a step away from leaking technical information from one layer into another. Eg your controllers don't care that they talk to a model. Your model doesn't care it talks to an sql or nosql database.

Another is about responsabilities. Does the user really need to do all of that? Is the user responsible for logging? It may be a correct choice, but it also seems that you ask it to do a lot of things. Does the user construct itself, update itself? etc

$this->model->user->id; // is it right way? Is not the right way. 1) This is very strong coupling. 2) do you need the id of the user? is the user only put in as dependency to get something from it? perhaps something like $this->user->showUserInfo($userInfoDisplay) can be used?

koen
  • 13,349
  • 10
  • 46
  • 51
  • 1) "take a step away from leaking technical information" - do you mean that I should use __construct(FrontPageModel $model) instead of just __construct($model)? 2) "Another is about responsibilities. Does the user really need to do all of that?" - To do *what*? :) I'm injecting object "User" because I need to know if current user is logged or not, how much rights he have for certain page etc. Yes it constructs itself and updates sometimes (rarely). – Roman Jan 15 '12 at 14:12
  • 1
    @oyatek 1) I suggest to use __construct(BusinessName $businessImplementation). What names would you use if your mother had to read the code and make sense of it? Would she care what level of your MVC layer she's reading about or that you are displaying a page that has some user info and a discussion thread? Your frontpage doesn't care either about those layers. It just wants to display UserInformation. 2) Checking whether the user is logged in is the responsibility of something else then the FrontPage. – koen Jan 15 '12 at 19:29