auraphp / Aura.Di

Dependency Injection System
MIT License
349 stars 63 forks source link

Add ability to track processed configs #95

Closed jakejohns closed 8 years ago

jakejohns commented 9 years ago
  • Add Container::addConfigured() and Container::hasConfigured() to allow checking if a ConfigContainer has been processed.
  • Implement this functionality in the ContainerBuilder.
  • Add tests for said function

Should there be some way to "track if a 'component' has been configured" when using the builder? Something like $di->hasConfigured('My\Package') ?

Here's a scenario.

My impulse is to allow the delegation of Component configuration to Foo and Bar, rather than require the consumer Project to know it needs Component\Config in addition to Foo\Config and Bar\Config. However, the separate Foo and Bar packages have only less than semantic means of knowing if Component has already been configured, and as such, would be wasting their time instantiating and processing the gratuitous config a second time.

I would think maybe the ability to do as follows would be useful:

namespace Component;

class Config extends ContainerConfig
{
    public function define(Contianer $di)
    {
        $di->set('component_service', $di->lazyNew('Component\Service'));
        // ... other stuff
    }
}
// In both Foo and Bar configuration would appear something like the following
// namespace Foo;
// namespace Bar;

class Config extends ContainerConfig
{
    protected $depends = [
        'Component\Config'
    ];

    protected $configs = [];

    public function define(Contianer $di)
    {
        foreach ($this->depends as $configClass) {
            if (! $di->hasConfigured($configClass)) { // check if configured
                $config = $di->newInstance($configClass);
                $config->define($di);
                $di->configured($configClass); // note config has been done
                $this->configs[] = $config;
            }
        }
        // ... other stuff
    }

    public function modify(Container $di)
    {
        foreach ($this->configs as $config) {
            $config->modify($di);
        }
        // other stuff
    }
}
namespace Project;

$di = $container_builder->newConfiguredInstance([
    'Foo\Config',
    'Bar\Config'
    // Don't need to add Component\Config here
    // Also, it will only be instantiated and processed once
]);

Currently, one could check if a service or params has been set, or something like that. eg:

class Config extends ContainerConfig
{
    protected $depends = [
        'component_service' => 'Component\Config'
    ];

    protected $configs = [];

    public function define(Contianer $di)
    {
        foreach ($this->depends as $provided => $configClass) {
            if (! $di->has($provided)) { // check for service
            // or maybe (isset($di->params['Component\Service']))
                $config = $di->newInstance($configClass);
                $config->define($di);
                $this->configs[] = $config;
            }
        }
        // ... other stuff
    }
}

I also considered storing the class names in the values array somehow, but all this seems off to me. I feel like what I want to know is "has the package been configured", not just if some particular service or param has been set and using values just seems abusive.

Am I off here? Should I not want to know that? Am I just in dependency hell already, if I feel like I want to know that? Does it not really matter because it's probably not that expensive to reconfigure the component anyway?

I was going to hold off on a PR until I was sure this made sense, or heard input, but it seemed simple enough to implement, so here it is. Note this doesn't stop you from processing the config twice if you want to by adding a check in the builder or anything. It just provides the ability to check if you so choose.

pmjones commented 9 years ago

/me furrows brow

On the one hand, it seems harmless. On the other hand (and on the gripping hand too) I do wonder why it's really needed. Are you in a situation where you need to check this regularly? Dependency checking in the dependency checking seems a bit much. Can you say more about the problem you're running into?

jakejohns commented 9 years ago

Ya, I think I might be off on this one. Shouldn't think out loud so much in PR's maybe, sorry.

I should have some code up soon (zeus willing) to illustrate, but I might work it out by then.

pmjones commented 9 years ago

@jakejohns Is this still something you feel is needed, or have you worked it out without it?

jakejohns commented 9 years ago

I can certainly live without it. As it seems no one shares my impulse, I should likely continue reworking some things.

pmjones commented 8 years ago

After long consideration, and seeing how the sentiment behind this PR seems not to be widely shared, I am closing the issue. We can reopen it later if needed.