5

Using pimple as my DI container, I have been bravely refactoring small classes to rely on DI injection, eliminating the hard-coded dependencies I could see would be easily removed.

My methodology for this task is very simple, but I have no clue if it is proper as I have very little experience with DI and Unit-testing aside from what I learned here in past month.

I created a class, ContainerFactory that subclasses pimple, and in that subclass have created methods that simply returns container for specific object.

The constructor calls the proper creator method depending on type:

function __construct($type=null, $mode = null){

 if(isset($type)){  
    switch ($type) {
      case 'DataFactory':
         $this->buildDataFactoryContainer($mode);     
        break;
      case 'DbConnect':
         $this->buildDbConnectContainer($mode);  
        break;
     default:
        return false;
    }
  }
}

The method signature for container object creation is as follows:

public function buildDataFactoryContainer($mode=null)

The idea being that I can set $mode to test when calling this container, and have it load test values instead of actual runtime settings. I wanted to avoid writing separate container classes for testing, and this is an easy way I thought to not have test related code all over.

I could instead have subclassed ContainerFactory ie: ContainerFactoryTesting extends ContainerFactory and override in that instead of mixing test code with app code and cluttering method signatures with $mode=null, but that is not the point of this post. Moving on, to create a container for a particular object, I simply do this:

 // returns container with DataFactory dependencies, holds $db and $logger objects.
 $dataFactoryContainer = new ContainerFactory('DataFactory');

// returns container with test settings.
$dataFactoryTestContainer = new ContainerFactory('DataFactory','test');

// returns container with DbConnect dependencies, holds dbconfig and $logger objects.
$dbConnectContainer = new ContainerFactory('DbConnect');

I just ran into an issue that makes me suspect that the strategy I am building upon is flawed.

Looking at the above, DataFactory contains $db object that holds database connections. I am now refactoring out this dbclass to remove its dependencies on a $registry object, but how will I create $dataFactoryContainer, when I add $db object that needs $dbConnectContainer?

For example, in datafactory container I add a dbconnect instance, but IT will now need a container passed to it ...

I realise my English is not that great and hope I have explained well enough for a fellow SO'er to understand.

My question is two-fold, how do you guys handle creating objects for dependencies that themselves contain dependencies, in a simple manner?

And .. how do you separate container configuration for creation of objects for testing purposes?

As always, any comment or link to relevant post appreciated.

Rodia
  • 1,407
  • 8
  • 22
  • 29
Stephane Gosselin
  • 9,030
  • 5
  • 42
  • 65
  • I have somewhat resolved the issue by creating my db object as so in datafactoryContainer creation method: $dbConnect = isset($dbConnect) ? $dbConnect : DbConnect::getInstance(new ContainerFactory('DbConnect')); – Stephane Gosselin May 26 '11 at 08:39
  • But I am pretty sure this is not using the DI container properly. – Stephane Gosselin May 26 '11 at 08:44
  • 1
    You should inject DataFactory into the constructor you showed. That code with switch block is not very DI-like. – Ondřej Mirtes May 26 '11 at 15:51
  • That's my name :) I recommend you to read Misko Heverys' articles which are a great guide how to write clean and testable code. http://misko.hevery.com/code-reviewers-guide/ – Ondřej Mirtes May 26 '11 at 16:11
  • @Ondřej Mirtes - FYi your username is untypable in most parts of the world! *(grin)*. As for the switch up there, it really didn't feel right when I put that together. Igors answer below and rereading the pimple doc page a few more times has made me totally change my approach. The object are now all initialised by closures in one container instead of being 'hardcoded' in multiple containers. Because of a trivial bug in pimple, on windows only I could not understand a proper usage scenario. After showing Igor, pimple was patched within minutes and I now use it the way it is meant to be used. – Stephane Gosselin May 26 '11 at 16:11

1 Answers1

6

You should not create separate containers for everything, but instead use a single container. You can create a container "Extension" which basically just sets services on your container.

Here is an extensive example of my suggested setup. When you have a single container, recursively resolving dependencies is trivial. As you can see, the security.authentication_provider depends on db, which depends on db.config.

Due to the laziness of closures it's easy to define services and then define their configuration later. Additionally it's also easy to override them, as you can see with the TestExtension.

It would be possible to split your service definitions into multiple separate extensions to make them more reusable. That is pretty much what the Silex microframework does (it uses pimple).

I hope this answers your questions.

ExtensionInterface

class ExtensionInterface
{
    function register(Pimple $container);
}

AppExtension

class AppExtension
{
    public function register(Pimple $container)
    {
        $container['mailer'] = $container->share(function ($container) {
            return new Mailer($container['mailer.config']);
        });

        $container['db'] = $container->share(function ($container) {
            return new DatabaseConnection($container['db.config']);
        });

        $container['security.authentication_provider'] = $container->share(function ($container) {
            return new DatabaseAuthenticationProvider($container['db']);
        });
    }
}

ConfigExtension

class ConfigExtension
{
    private $config;

    public function __construct($configFile)
    {
        $this->config = json_decode(file_get_contents($configFile), true);
    }

    public function register(Pimple $container)
    {
        foreach ($this->config as $name => $value) {
            $container[$name] = $container->protect($value);
        }
    }
}

config.json

{
    "mailer.config": {
        "username": "something",
        "password": "secret",
        "method":   "smtp"
    },
    "db.config": {
        "username": "root",
        "password": "secret"
    }
}

TestExtension

class TestExtension
{
    public function register(Pimple $container)
    {
        $container['mailer'] = function () {
            return new MockMailer();
        };
    }
}

ContainerFactory

class ContainerFactory
{
    public function create($configDir)
    {
        $container = new Pimple();

        $extension = new AppExtension();
        $extension->register($container);

        $extension = new ConfigExtension($configDir.'/config.json');
        $extension->register($container);

        return $container;
    }
}

Application

$factory = new ContainerFactory();
$container = $factory->create();

Test Bootstrap

$factory = new ContainerFactory();
$container = $factory->create();

$extension = new TestExtension();
$extension->register($container);
igorw
  • 27,759
  • 5
  • 78
  • 90
  • Igor! What a small world this is. I was looking for a way to reach you by email and here you come answering my SO question! Weird concidence! I have been on your github account, landed there while looking for info on pimple. I made a mental check to revisit silex as it look very interesting. The documentation has an example given that generates a fatal error when I tried to recreate it with dummy classes. I did find the solution to wrap with protected(), but I still am unclear on a concept or 2 concerning pimple, I really wish I had your email somehow but SO is way better than nothing. – Stephane Gosselin May 26 '11 at 10:06
  • Thanks for taking the time, and the great response. **Very** much appreciated M. Wiedler – Stephane Gosselin May 26 '11 at 10:12
  • _This post_ of yours would make for a very fine addition to the existing pimple documentation. The documentation available is not bad at all but overriding for testing wasn't clear, nor was the usage of an object with a dependency inside another object. And the ContainerFactory that ties everything together is really the icing on the cake. It pretty much answered all the 'how do I make Pimple get along with this app' questions I had. My 2 cents and kudos to you. PS: The commit you pushed today works great on my end. Thanks. – Stephane Gosselin May 26 '11 at 18:02
  • Yeah, some more documentation for Pimple would be nice. Quite a lot is covered in the Silex docs: http://silex-project.org/doc/services.html Pretty much everything from that chapter applies to Pimple itself. – igorw May 26 '11 at 18:26