container-interop / service-provider

[EXPERIMENTAL] Promoting container/framework interoperability through standard service providers
72 stars 10 forks source link

split ServiceProviderInterface in two interfaces? #43

Closed oscarotero closed 11 months ago

oscarotero commented 7 years ago

The v4.0 release brings a interface with two methods: getFactories() and getExtensions(). I'm agree with that change but suggest to create two different interfaces, for example ServiceProviderInterface and ServiceExtensionProviderInterface to avoid the need to create empty methods just for the interface compliance (example: https://github.com/oscarotero/folk/blob/master/src/Providers/Router.php#L35)

mnapoli commented 7 years ago

Hi! Honestly it's simpler for everybody:

So it's not a huge impact either way but it seems simpler to me to have just one interface.

oscarotero commented 7 years ago

IMHO, factories and extensions are different things and should be treated differently. For example, a library can provide a factory to easily create instances, and can contain plugins providing extensions to register themselves into the instance. Example:

//Register library
$container->addFactories(new Foo\ServiceProvider());

//Register some plugins
$container->addExtensions(new Foo\Plugins\One\ExtensionServiceProvider());
$container->addExtensions(new Foo\Plugins\Two\ExtensionServiceProvider());

Other use case is to extends some services under specific contexts:

if (getenv('ENV') === 'dev') {
    $container->addExtensions(new Foo\Plugin\DevTools\ExtensionServiceProvider());
}
jakejohns commented 5 years ago

I agree it would make sense to split into a "Defines" and "Modifies" interface. Otherwise, an abstract "null" provider seems like it might be useful, then you can just extend and override the desired method and not have to worry about writing the empty one.

abstract class AbstractProvider implements ServiceProvider
{
    public function getFactories() : array
    {
        return [];
    }

    public function getExtensions() : array
    {
        return [];
    }
}
XedinUnknown commented 5 years ago

I think the new interface with getFactories() and getExtensions() is precisely what we needed. Before this change, I did not contribute to this standard, because when I got to implement my own container-based module architecture, the service provider appeared useless. It only had one method, which returned a plain list of callables that accepted ambiguous arguments. It was possible to implement a container hierarchy simply by using lookup delegation, and introducing a standard (this one) that didn't have benefits would have been a useless complication. But not any longer! Thanks! :)

mindplay-dk commented 11 months ago

Having two methods does not appear to solve a problem, it's more like just moving the problem somewhere else, now requiring run-time type-checking.

Based on the comments by @mnapoli and @XedinUnknown I'm going to close this issue.