container-interop / service-provider

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

Swap arguments order in ServiceProviderInterface::getExtension callables? #50

Open romm opened 4 years ago

romm commented 4 years ago

It seems to me that the order of arguments given in ServiceProviderInterface::getExtension callables would make more sense if they were swapped.

I've been using this kind of extensions quite often:

public function getExtensions(): array
{
    return [
        MyServiceInterface::class => static function (ContainerInterface $container, MyServiceInterface $myService): MyServiceInterface {
            return new AggregateOfMyService(
                $myService,
                new SomeOtherImplementationOfMyService()
            );
        },
    ];
}

In this example, the instance of ContainerInterface is not used, leading to an unused variable and making static analysis tools (including IDE) complain about it (which is right IMO).

When I think about it, I believe that in extensions, the former value of the service should always be present in parameters (even if it's null) because the callback should extend this value, whereas the container is not always needed (see example above).

What about changing the callable signature to:

function(?$previous, ContainerInterface $container) : mixed

Some would maybe argue that this should be a valid use-case:

public function getExtensions(): array
{
    return [
        MyServiceInterface::class => static function (ContainerInterface $container, ?MyServiceInterface $myService = null): MyServiceInterface {
            return new SomeOtherImplementationOfMyService(
                $container->get('some_dependency')
            );
        },
    ];
}

But I don't agree: in this case, a factory should be prefered over an extension.

WDYT?

antonpresn commented 2 years ago

Also, it's quite hard to implement (and maybe even impossible) with concrete container (such as https://php-di.org/), which uses decoration parameters as suggested here (when decorated value goes first)

mindplay-dk commented 11 months ago

When I think about it, I believe that in extensions, the former value of the service should always be present in parameters (even if it's null) because the callback should extend this value, whereas the container is not always needed (see example above).

My pattern-loving programmer brain screams "NOOOOO", but I do see the point.

Technically, the factory and extension callables are unrelated types, so although consistency would feel good here, it's not technically required, I think.

Also, it's quite hard to implement (and maybe even impossible) with concrete container (such as https://php-di.org/), which uses decoration parameters as suggested here (when decorated value goes first)

@antonpresn I don't know what is meant by "decoration parameters" here? Why would the signature of callable types in a PSR service provider affect the concrete container?

I'm leaving this issue open for discussion.