[PSR-11] Review: remove the sentence about optional "get" parameters

[David Négrier]

This is a followup to Larry's message here: https://groups.google.com/d/msg/php-fig/L8rDUwRFsOU/AmNE1PlFBgAJ

The spec says:

While ContainerInterface only defines one mandatory parameter in get(), implementations MAY accept additional optional parameters.

Larry says:

* The allowance for additional parameters on get() is worrisome. That implies ANY use of a second or further parameter sets me up for an incompatibility, but implementations are free to add those implementation-specific hooks that I can couple to.  I would much prefer to omit that allowance entirely.  (Technically optional parameters cannot be prevented, but we shouldn't encourage it.)


When we first released container-interop, we wrote this to tell it was ok to add support for ContainerInterface to already existing containers that did accept more than one parameter for the "get" method (for instance: http://api.symfony.com/2.3/Symfony/Component/DependencyInjection/ContainerInterface.html)

Yet, I do agree with Larry. This does not belong to a PSR. Maybe we can simply add a comment about this in the META doc and completely get rid of this sentence.

I'm +1 to remove the whole sentence.
Anyone has any objection? If not, I'll write a PR.

++
David.

[Matthieu Napoli]

Yes the point is that many containers have optional extra parameters, but ContainerInterface is still compatible with them so all is fine (i.e. there is no incompatibility).

So just to get this straight: we remove the sentence (because it sounds weird to have it in a standard), but we agree that it's still completely fine if implementations have extra *optional* parameters right? Because we obviously want to keep all containers compatible.

If so, I'm +1 to remove the sentence (I just don't want to force implementations to have exactly 1 parameter, which doesn't make sense in PHP and in OOP in general).

Matthieu

[Niklas Keller]

So just to get this straight: we remove the sentence (because it sounds weird to have it in a standard), but we agree that it's still completely fine if implementations have extra *optional* parameters right?

If it were completely fine, we wouldn't remove the sentence, no? Optional parameters are *not* fine. It hurts interoperability. If a consumer passes them, it assumes a concrete implementation. If the implementation is swapped, they might be ignored, but it's even worse if that implementation also has optional parameters, then they might be interpreted wrongly.

Because we obviously want to keep all containers compatible.

They *are* still compatible, because these parameters are *optional*, so don't have to be provided. 

I just don't want to force implementations to have exactly 1 parameter, which doesn't make sense in PHP and in OOP in general

Yes, it does make sense. If you only code to interfaces, optional parameters that are not defined in the interface do not make sense. 

[Matthieu Napoli]

Hi Niklas, I think there is a misunderstanding.

- if you use the interface, then you use the parameters defined in that interface => so yes, you should NOT pass extra parameters (we agree on that, that doesn't make sense)

- if you don't use the interface but the implementation directly, then it's OK to use the extra parameters (because you code against the implementation)

That's why OOP (and PHP) allows implementations to have more parameters than defined in the interface (because you stay compatible with the interface).

Now we usually say "code against interfaces", because obviously that's much better. But here for container interoperability we have to consider the fact that not all frameworks can switch containers entirely. So some pieces of code will still use the implementation, that's why we have to be compatible with this.

In any case, it doesn't make sense that an interface forbids extra optional parameters, because again OOP and PHP allows it. The question is whether we want to make that explicit or not in the document: I usually like explicit because it removes confusion for implementors (DIC authors).

Matthieu

--

You received this message because you are subscribed to a topic in the Google Groups "PHP Framework Interoperability Group" group.

To unsubscribe from this topic, visit https://groups.google.com/d/topic/php-fig/zY6FAG4-oz8/unsubscribe.

To unsubscribe from this group and all its topics, send an email to php-fig+u...@googlegroups.com.

To post to this group, send email to php...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/8dc6ee8a-7459-4a29-91d3-6ea6fa028c69%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[Pedro Cordeiro]

If one relies on passing other parameters to the container on their consumer implementation, then they're not coding to a generic interface, but to a specific container implementation. So, this PSR has no business saying what he can or can't do.

Maybe we could specify that if implementations allow for additional parameters, they still need to be able to return a service even if only the first parameter (the service name) is passed. This way, we don't end up with PSR-11 implementations that NEED 2+ parameters to work.

[Larry Garfield]

Suggestion: Make no comment in the spec itself, but note in the metadoc that "some existing implementations have extra optional parameters; that's technically legal for backward compatibility but discouraged".  So that way Symfony's DIC, for instance, can remain compatible but anyone writing a new implementation knows they're not supposed to do that.

--Larry Garfield

On 11/03/2016 03:42 AM, Matthieu Napoli wrote:

[David Négrier]

Ok, I tried a synthesis of Matthieu's and Larry's point of view in this PR: https://github.com/php-fig/fig-standards/pull/831

Let me know if you find it acceptable.

David
--
Twitter: @david_negrier
Github: @moufmouf