[Contracts][DependencyInjection] Add ServiceCollectionInterface
#19370
Conversation
3bbf43a to
a1eb47f
Compare
|
Thanks for your suggestions Kevin! I reworked the PR, is it better now? 🙂 |
| foreach ($this->locator as $serviceId => $service) { | ||
| // do something with the service, the service id or both | ||
| } | ||
|
|
||
| // ... | ||
| $handler = ($this->locator)($commandClass); |
There was a problem hiding this comment.
ServiceLocator is indeed callable but this isn't part of the contracts:
public function someAction(ServiceCollectionInterface $locator)
{
$handler = ($locator)($commandClass); // this works but static analysis will complain
}I'd prefer removing this in favor of $this->locator->get($commandClass) but as we've shown this callable example for some time, I'm not sure... @nicolas-grekas, what do you think. I don't have the context on why ServiceLocator was made callable initially.
There was a problem hiding this comment.
I would reformulate the whole paragraph. Why tell that the injected service is some implementation-specific class while only an interface is type hinted? This is a bad hint that encourages against SOLID principles.
There was a problem hiding this comment.
Alright, I tried something totally different not mentioning "internal" implementation but only explaining the locators are countable and traversable.
a1eb47f to
5a6313b
Compare
| which implements both the PSR-11 ``ContainerInterface`` and :class:`Symfony\\Contracts\\Service\\ServiceProviderInterface`. | ||
| It is also a callable and a countable:: | ||
| The injected service is an instance of | ||
| :class:`Symfony\\Component\\DependencyInjection\\ServiceLocator`. A service |
There was a problem hiding this comment.
I still wouldn't mention this, it's an implementation detail.
We should tell about ServiceCollectionInterface: if ppl type-hint for it, then they will get an object that is coutable+iterable, in addition to being a PSR11 locator.
5a6313b to
dcd25e7
Compare
|
I tried to remove useless implementation details and go for the type-hint approach Nicolas suggested. |
|
A nice contribution! Thanks a lot Alex and thanks for your patience during the review 🙏 Thanks also to Kevin and Nicolas for the review! |
Fix #19368
Ping @kbond 🙂