container-interop / service-provider

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

Change method signature to return `iterable`? #49

Closed romm closed 8 months ago

romm commented 4 years ago

This is only a quick thought, maybe there would be some side-effects that I can not see right now.

Changing the return type of the two methods to iterable would ease the use of generators, for instance something like this:

final class AggregateServiceProvider implements ServiceProviderInterface
{
    /** @var ServiceProviderInterface[] */
    private array $serviceProviders;

    public function __construct(ServiceProviderInterface ...$serviceProviders)
    {
        $this->serviceProviders = $serviceProviders;
    }

    public function getFactories(): iterable
    {
        foreach ($this->serviceProviders as $serviceProvider) {
            yield from $serviceProvider->getFactories();
        }
    }

    public function getExtensions(): iterable
    {
        foreach ($this->serviceProviders as $serviceProvider) {
            yield from $serviceProvider->getExtensions();
        }
    }
}

What do you think?

bnf commented 4 years ago

I like the idea and thought about proposing that as well, there is one thing I wasn't sure about:

maybe there would be some side-effects that I can not see right now

There is one drawback of iterable in constrast to array for consumers (containers): An iterable can only be iterated once, as a generator may be closed after the first iteration. That also means: No array-style access nor copy operations with + .

Still, I think iterable would be good. The spec should make it cristal clear that consuming containers should be unit tested against generators to ensure they do only perform iterations on getFactories() and getExtensions(), and only once.

mindplay-dk commented 9 months ago

There is one drawback of iterable in constrast to array for consumers (containers): An iterable can only be iterated once, as a generator may be closed after the first iteration. That also means: No array-style access nor copy operations with + .

This sounds like it might invite problems?

Why would you need generators for this? What's the advantage over arrays?

Aren't containers basically always going to apply the entire set of factories and extensions all at once?

mindplay-dk commented 8 months ago

Since this idea appears to invite problems, I am going to close this issue. No real reason was given for this suggestion. If you did have generators for some reason, since they are going to be consumed by the container in full anyway, you can simply convert them to arrays. I would imagine this is a very rare issue anyway, as generators are typically used for sequential data rather than sets/maps.

If you disagree, please feel free to reopen and clarify the requirement.