Remove the ServiceProvider interface and replace it with an array of callables

[mnapoli]

Instead of:

use Interop\Container\ServiceProvider;

class MyServiceProvider implements ServiceProvider
{
    public function getServices()
    {
        return [
            'my_service' => function(ContainerInterface $container, callable $getPrevious = null) {
                $dependency = $container->get('my_other_service');
                return new MyService($dependency);
            }
        ];
    }
}

We now write:

$provider = [
    'my_service' => function (ContainerInterface $container, callable $getPrevious = null) {
        $dependency = $container->get('my_other_service');
        return new MyService($dependency);
    }
];

Now that we allow callables as factories I don't see the point to having an interface. Discovery of service providers (i.e. PHP arrays) can be handled in many ways without affecting containers (Puli, file returning the array, class/function/method returning the array, etc.).

Also fixes #19

[moufmouf]

Ok, I've been spending the last 2 days thinking hard about this.

First of all, let me say that the "genius" part of the service-providers standard is the signature of the factory (i.e. function(ContainerInterface $container, callable $getPrevious = null)). This is really the core of the proposal. Everything else is details. So I understand your idea of restricting the proposal to the core of it.

My first reaction was to say to myself: "Hey, but we need this interface!". But this was also my first reaction when we spoke about the delegate lookup feature in container-interop, and it turned out I was wrong. So I've learned to be careful of my first impressions.

Yet, I see some points that trouble me. Here are my thoughts.

If we say that a service provider is simply an array of factories, there are really 2 alternatives. Either the array of factories is provided "as-is" to the container, or the container knows somehow how to retrieve this array of factories.

Alternative 1: the container is provided the array of factories

In this scenario, each container has a method that looks like this:

$container->setProvider([
    'entry' => function() { ... }
]);

// or

$container = new Container([
    'entry' => function() { ... }
]);

We really don't care about the name of the method.

The problem with this approach is that the container MUST be fed with the array (i.e. the array must be resolved for each query). On the contrary, using the ServiceProvider interface, the container can decide whether to call getServices or not (depending on whether the services are actually fetched or not, or on its cache or whether he could compile the definition).

So basically, if we go with a setProvider method, containers cannot lazily load the factory array. This is clearly a problem for me.

Of course, we could solve this by saying that a service provider is a callable that returns an array of factories.

$container->setProvider(function() { return [
    'entry' => function() { ... }
]});

I'm not sure how I feel about this. It's pretty close to what we do with the interface. The interface clearly states what we expect (it carries meaning and allows us to do type hinting). For lazy guys, we can always create an anonymous class implementing it (it seems we wont be able to use functional interfaces though).

Alternative 2: the container delegates fetching of the array to some other mechanism

We don't want the array of factories to be constructed on each request. So the container must know how to fetch that array (to be able to build it lazily). I agree this does not need to be a getServices method, but if we want the container to construct this object, it must know how to build it.

Since there are many containers out there, we should definitely define a way for the container to fetch that array.

We could very well define that the array of factories are stored in files instead of classes. For instance:

return [
    'entry' => function() { ... }
];

I understand the appeal of this solution (it mimics services.yml files, it can be discovered using Puli easily...)

What is important though is that I think we need to be specific. If we don't tell upfront in this standard how a container can fetch the array of factories (by calling a method or including a file...), we are facing interoperability problems between containers that would not know how to fetch the array of factories.

[mnapoli]

If we don't tell upfront in this standard how a container can fetch the array of factories […] we are facing interoperability problems […]

Right agreed. Here is a list of options:

• file returning an array
• class method returning an array
• callable returning an array
• … ?

Any reason to prefer one or another?

[moufmouf]

Any reason to prefer one or another?

I'll need to think a bit more about it. In the meantime, here is a 4th possible option:

• class implementing Traversable and ArrayAccess

[moufmouf]

Ok, let me give a try at pros and cons of each option we detected so far.

Callable returning an array

$container->setProvider(function() { return [
    'entry' => function() { ... }
]});

Pros

• The simplest thing we can get to write test service providers (no need to create a new file or a new class)

Cons

• I have no idea how this could be discoverable (by Puli or any other discovery mechanism).

File returning an array

my.provider.php

<?php
return [
    'entry' => function() { ... }
];

$container->register('my.provider.php');

Pros

• Mimics the idea of a configuration file (like services.yml, etc...)
• Easily discoverable through Puli (or other mechanism). For instance, we could have a convention to find all providers matching path "*/.provider.php"

Cons

• Promotes the use of closures VS static public functions (because it is way easier to write a closure in the file than to create a separate class containing the factories). Hence, it will lead more naturally to entries that cannot be easily optimized in compiled/cached containers like Yaco/PHP-DI.
• Cannot be configured. We are unable to modify the behaviour of the service provider by passing parameters in the constructor (to be honest, I'm not sure this is really a bad thing)

Class method returning an array

Pros

• Close to the idea of a service provider available in many frameworks (Laravel, Silex...)
• Autoloadable via solid / well-understood mechanism (PSR-0 / PSR-4) => no need to wonder what the relative path of the service provider file is (or go through Puli to have a virtual path)
• Easier to promote the use of static public functions (it still needs to be promoted but it is more natural than using a file)
• Can be easily tested without creating a new file using an anonymous class:

$container->register(new class implements ServiceProvider() {
    public function getServices() {
        return [
            'entry' => function() { ... }
        ];
    }
});

• Discoverable (via Puli or other mechanism)

Cons

• Creating a class is slightly longer than creating a file (a few more lines of code)
• For discovery to work, we need to assume the class has a constructor with no compulsory argument. This way, an instance of the service provider can be created easily.
• Factories are essentially using a functional paradigm (all needed parameters for the result are passed in parameters). Only in rare case can attributes from the object be useful. I understand how using a class to hold those function can be confusing (especially for OO folks)

Class implementing Traversable and ArrayAccess

The idea here is to have an object that behaves as an array of factories. It can be fed to a container just like a regular array of factories (but can be lazily evaluated).

Pros

• Containers can register simple arrays:

$container->register([
            'entry' => function() { ... }
]);
// or
$container->register(new MyServiceProviderThatBehavesLikeAnArray());

Cons

• Performance-wise, ArrayAccess/Traversable must be way slower than a simple array.
• Hard to implement (for service provider authors)

My choice

If I have to make a choice, I'd clearly eliminate the "Callable returning an array" solution, because I don't see how discovery can work with that.

"Class implementing Traversable and ArrayAccess" seems a bit an overkill and is hard to implement. I'll eliminate it too.

This leaves us with files VS classes. I can live with both solutions, although I have a preference for the class because:

• performance-wise (it will be easier to promote public static functions)
• it is closer to what people expect from a service provider (so easier to get adopted)
• it is slightly more flexible
• I can live with the fact that we must have a constructor with no argument for discovery to work

I'd be curious to hear what others have to say on this topic.

[shochdoerfer]

Personally I would love to see the "Class method returning an array" approach. Mainly because no one needs to deal with any kind of file (auto)loading. It will simly work due to PSR-0 or PSR-4. Compared to the file method there is not really much more "boilerplate" code we need to add.

[mnapoli]

To sum up:

• "Callable returning an array" is ruled out easily
• "Class implementing Traversable and ArrayAccess" can be ruled out because of performance issues and being complex to implement, for no benefit over arrays

Simply saying "an array of callables" makes interoperability hard because we have no "standard" way to discover the arrays, so the whole system is harder to use.

We are left with the current solution (method returning an array) and "file returning an array". I agree that if we want to encourage using static methods then an array of callables is not helping (since we would have to create the array of callables + the class containing the factory methods) => closures would be favored by implementors.

So right now I tend also to the same conclusion: the class with getServices() offers a few advantages, even though it's a little less simple. I'll close this issue for now, feel free to reopen the discussion if necessary. Thanks for the discussion!