Is this a sane implementation of constructor injection?

Question

Following on from my question on service locators I have decided to use constructor injection instead. Please consider the following code:

<?php

    interface IAppServiceRegistry {
        public function getDb();
        public function getLogger();
    }

    interface IFooServiceRegistry extends IAppServiceRegistry {
        public function getFooBarBazModel();
    }

    class AppServiceRegistry
        implements IAppServiceRegistry, IFooServiceRegistry
    {
        private $logger;
        private $db;
        private $fooBarBazModel;

        public function getDb() {
            // return db (instantiate if first call)
        }

        public function getLogger() {
            // return logger (instantiate if first call)
        }

        public function getFooBarBazModel() {
            if (!isset($this->fooBarBazModel)) {
                $this->fooBarBazModel = new FooBarBazModel( $this->getDb() );
            }
            return $this->fooBarBazModel;
        }
    }

    // Example client classes:

    /**
     * Depends on db, logger and foomodel.
     */
    class Foo {
        private $db;
        private $logger;
        private $fooModel;

        public function __construct(IFooServiceRegistry $services) {
            $this->db = $services->getDb();
            $this->logger = $services->getLogger();
            $this->fooModel = $services->getFooModel();
        }
    }

    /**
     * Depends on only db and logger.
     */
    class BarBaz {
        private $db;
        private $logger;

        public function __construct(IAppServiceRegistry $services) {
            $this->db = $services->getDb();
            $this->logger = $services->getLogger();
        }
    }

Then I would add new service factory methods to the registry as the application evolved, creating segregated interfaces wherever logically appropriate.

Is this approach sane?

Answer 1

Although I don't normally read php, I think I understand most of it. Technically, it looks fine, but then you write

I would add new service factory methods to the registry as the application evolved

That tends to hurt the idea of loose coupling because instead of a general-purpose Service Locator you now have a specialized Service Locator.

Injecting such a serivce registry into each class works against the Single Responsibility Principle (SRP) because once a class has access to the registry, it is too easy to request yet another dependency than originally conceived, and you will end up with a God Object.

It would be better to inject the required dependencies directly into the classes that need them. Thus, your Foo class' constructor should take a db, a logger and a fooModel, while the BarBaz class should take only a db and a logger parameter.

The next question may then be: What if I need a lot of different dependencies to perform the work? That would require a truly ugly constructor with lots of parameters, and this goes against other well-known OO practices.

True, but if you need lots of dependencies, you are probably violating the SRP and should try to split your design into finer-grained objects :)

Answer 2

This implementation is almost the same as the service locator you previously showed us.

A good question to ask is whether upon looking at the class of your objects, you know everything about the objects needs to get their job done. In your case you still don't know.

If Foo needs the db, logger and model, you make this clear by asking for these in the constructor.

Here's a good read on the matter:

http://misko.hevery.com/code-reviewers-guide/flaw-digging-into-collaborators/

Answer 3

I've been grappling with this kind of problem lately. Service Locator vs Depedency Injection.

I agree with Mark that the way to go is to inject individual fine-grained objects into the constructor, as required. The only drawback, as Mark has highlighted, is that when you build a complex object graph, you inevitably have to start somewhere. This means that your high-level objects will have lots of services (objects) injected into them.

The easy way round this is to use something to do the hard work for you. The prime example of this is Google's Guice which seems to me a very good way of going about things. Sadly it's written for Java! There are PHP versions about; I'm not sure any of them quite measure up to Guice at this point.

I've written a post on this topic which goes into more detail; which you may find interesting. It includes a simple implementation of a dependency injection framework.

The bottom line is that if you've got a class Foo with a number of requirements, you can create your class as such:

/**
 * Depends on db, logger and foomodel.
 */
class Foo
{
    private $db;
    private $logger;
    private $fooModel;

    /**
     * (other documentation here)
     * @inject
     */
    public function __construct(IDbService $db, ILoggerService $logger, $iModelService $model)
    {
       // do something
    }
}

When you want a new Foo object, you simply ask the dependency injection framework to create you one:

$foo = $serviceInjector->getInstance('Foo');

The dependency injector will do the hard work, making sure it injects the dependencies. This includes any dependencies of the dependencies, if that makes sense. In other words it will sort it all out recursively up the tree.

Later, when you find you need an IBarService object, you can just add it to the Construtor, without altering any other code!