[REVIEW] PSR-11 Container Interface

[Matthew Weier O'Phinney]

Greetings, on this last day of 2016! (For some of you, it's already 2017!)

I'm rebooting the REVIEW period for PSR-11 as of now; review will end
at 11:59 on 13 January 2017, with the possibility of starting a vote
under the existing FIG 2.0 by-laws possible immediately thereafter.

The changes since the previous review period started include:

- All exceptions were moved into the `Psr\Container` namespace
(instead of the `Psr\Container\Exception` namespace as was done
previously).

- Clarifications were added around inclusion of OPTIONAL parameters to
the `get()` method by _implementing libraries_, noting that doing so
is allowed by PHP, but discouraged, due to ambiguity in behavior
between implementations.

- Added a formal definition of an entry identifier for use with
`get()` and `has()`, noting that these may be any valid non-empty
string.

- Removed all verbiage around the delegate lookup feature. Consensus
was that this is an interesting feature, but something that may be
implemented by libraries while still following the specification. If
anybody feels the need to formalize this detail, it can be done in a
later specification without breaking compatibility with the current
proposal.

We have also rejected the proposed
`MisconfiguredServiceExceptionInterface`. David has previously noted
reasons not to include it
(https://groups.google.com/d/msg/php-fig/2pnhudRUpQg/ewoGoNtFCgAJ),
which received no rebuttal. We feel it does not add value to consumers
currently, would likely be problematic in terms of performance, and
dictates too many details around implementation.

Please review the specification again at this time, and let us know as
soon as possible if you see any potential problems that remain to be
addressed.

--
Matthew Weier O'Phinney
mweiero...@gmail.com
https://mwop.net/

[Chuck Burgess]

LGTM from PEAR...

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+unsubscribe@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/CAJp_myU%2BwCPNbmhdMGztNvzmgrjpO-giGBE8fuuJT4Lj%3DDVVGg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

[Stefano Torresi]

Hey folks,

To avoid the usual insurgence of concerns and critiques that only happen when an acceptance vote starts, and which to be really addressed would require to first cancel the vote, then push back the spec to draft, then start reviewing again, I would urge anybody who thinks that the this spec is not up to par to speak their mind before Friday the 13th! To accomplish this, I would also suggest to spam this thread all around your favourite mediums.

That said, I personally think the spec is sound. I'm sure this PSR will be a huge success, and will set an important precedent regarding prior-to-acceptance adoption of PSR drafts, paving the road for future WGs.

See you around.

LGTM from PEAR...

To unsubscribe from this group and stop receiving emails from it, 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/CAJp_myU%2BwCPNbmhdMGztNvzmgrjpO-giGBE8fuuJT4Lj%3DDVVGg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--

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

To unsubscribe from this group and stop receiving emails from it, 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/CANsgjnuVHjWScCCkuuf5LkT7OHXZAa%3DmvyvRVMB4sC%3DXLOhcVA%40mail.gmail.com.

[David Négrier]

Hey folks,

Before the review ends, I'd like to update the list of contributors in the META document.

I'm proposing a PR to add Larry Garfield to the list of contributors as he made major feedback and contributed a PR to PSR-11:

- https://github.com/php-fig/fig-standards/pull/864

If any of you see other people that contributed and are missing from the contributors list, please tell us in this thread, or even better, submit a PR: https://github.com/php-fig/fig-standards/blob/master/proposed/container-meta.md#93-contributors

Regards,
David.
Twitter: @david_negrier
Github: @moufmouf

[Larry Garfield]

Late in the week, I know, but this is when I had time. :-(

One:

" A call to get can trigger additional calls to get (to fetch the
dependencies). If one of those dependencies is missing,
the NotFoundExceptionInterface triggered by the inner get call SHOULD
NOT bubble out. Instead, it should be wrapped in an exception
implementing the ContainerExceptionInterface that does not implement
theNotFoundExceptionInterface."

I still find this language unacceptably vague and backward. "Throw
something implementing X but not Y which is a child of X" is needlessly
clumsy. There was, previously, a suggestion to define a
"MissingDependencyException" to use specifically, which is more targeted
than "MisconfiguredServiceException" (and avoids the "breaks how PHP
works" issue that David noted previously). At the very least specifying
that as the exception to throw (which is defined, and therefore we know
it doesn't extent NotFoundException) the above language becomes much
simpler, and for those who do wish to catch it there's a very clear
known value to catch.

I will be honest, I find the idea of "Don't define an exception for a
known error case because *I* can't think of a reason you'd catch that"
to be spurious at best. It provides for more robust error handling in
some cases, and zero harm in any case. Even if you could argue the
justification for it is small, the justification against it is
non-existent (when limited to just missing dependencies).

Two:

I understand the reasons why the delegate lookup was removed outright,
and I do not dispute them. However, the net result is that we have an
interface that is almost trivial, on a topic that is of questionable
appropriateness in the first place. (I still question the validity of a
container interface at all.) That is, with that removed, what's left to
standardize? get() and has() are barely an specification.

Three:

There's a formatting issue in the metadoc, 9.3, contributors: Taylor
Otwell is separated from the rest of the group for some reason. I'm
sure there's nothing to be read into that. :-)

--Larry Garfield

[David Négrier]

Hey Larry,

Let me start by answering your second comment.

The fact that the "delegate lookup feature" was removed from PSR-11 does not make PSR-11 an empty shell. PSR-11 is already useful with or without it. Just have a look at Slim 3, Zend Expressive, Behat or any of the consumers of ContainerInterface for appropriate use cases.

- It is useful for routers to fetch a controller from the container.
- It is useful for writing adapters extending any containers.
- It is useful for composing containers.
- It is useful for writing factories that do not depend on a particular container.
- In general container-interop has proved the usefulness of the ContainerInterface so it's a shame questionning that again on the last day of the review

The delegate lookup feature was mainly removed on the ground that it is a design pattern and therefore not well suited for a PSR. (as a reminder, it's simply the ability for a container to delegate the lookup of dependencies to another container). Nothing prevents any of the containers out there to implement this design pattern while still implementing PSR-11.

Also, I'd like to make a parallel with PSR-7 and PSR-15. PSR-7 is already useful by itself, but it will be immensely more useful when PSR-15 passes. It is the same with PSR-11. It will be the cornerstone of interoperable service providers. As soon as PSR-11 passes, we plan forming a working group on standardizing service providers (see https://github.com/container-interop/service-provider/ for more details). PSR-11 is a prerequisite.


Regarding your comments on exceptions, I know we disagree on this specific topic.

You say:

> I find the idea of "Don't define an exception for a
> known error case because *I* can't think of a reason you'd catch that"
> to be spurious at best.

I'd rather say: "Don't standardize an *exception interface* for a known error case because *nobody* can think of a reason you'd catch that"

Matthieu asked time and time and time again for a good reason to catch it. Collectively, on the PHP-FIG mailing list, *nobody* was able to give a good use case to catch it. And in more than 2 years of "container-interop" usage, noone ever asked for it. So I'm pretty sure it's safe to say there is no need for it.


Regarding your last issue:

> There's a formatting issue in the metadoc, 9.3, contributors: Taylor
> Otwell is separated from the rest of the group for some reason.  I'm
> sure there's nothing to be read into that. :-)

Poor Taylor! :) I've looked at the markdown file and can't find anything weird there. Unless someone can point me to a mistake I did not see, I believe the error spurs from Github markdown parser that is adding an additional <p> in the <li> of Taylor for no reason. This does not seems to happen with other markdown parsers... anyone would have a workaround for this?

++
David.

[Chuck Burgess]

On the exceptions, that language just says to me that in my get() method, I have a try/catch around any deeper get() calls, and thus my outer get is going to catch *anything* and wrap it with the package-level exception before throwing it.  It's not really about the source exception being NotFound... it's about the package-level exception behavior in general.

Maybe that language should be more generic regarding the inner exception?  Maybe the use pattern should be presumed rather than specified.  

Then again, maybe the fact that the exception that comes out *is* a package-level exception should be good enough, since even the NotFound *will* be caught by catching the package-level exception.  If that was actually your point, Larry, I think I can accept that.  This has highlighted to me that a package *sub-level* _interface_ is not the same animal as a package sub-level *concrete* exception class, and requires a different viewpoint when seeing the runtime scenarios.

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

To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+unsubscribe@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/a77b3d96-2edf-e5ad-4cc1-d41ffaa9a9b1%40garfieldtech.com.

[Nicolas Grekas]

Hi all,

for the shake of trying (meaning I come here with no announcement at all that this is going to be merged), here is what this currently looks like on the Symfony container:
https://github.com/symfony/symfony/pull/21265

the exception part is really where things start to look like a zoo. Because we have many animals there in the "Exception" namespace, and mapping them to PSR11's NotFoundExceptionInterface is going to be non trivial and never perfect I'd say.

If you read the thread, you'll see that this sentence "Exceptions directly thrown by the container" is unclear for now - what does "directly" mean?

the NotFoundExceptionInterface triggered by the inner get call SHOULD NOT bubble out.

Fortunately enough, this is only a "SHOULD NOT", so we can do as we want and still be compliant. Because I agree with Larry here, the wording is so abstract that in practice it hardly helps. What does missing mean? Eg does a brokenly configured first level service qualify as such?

Reasoning from the inside of the container, you may say no.

But reasoning from the outside, the user might expect a yes, so that eg a chained-container can continue with the next container in the chain (provided NotFoundExceptionInterface VS ContainerExceptionInterface could be the trigger to decide if the chain should break or continue? what is it for otherwise? doesn't that concept duplicate the "has()" method?)

 

"Don't standardize an *exception interface* for a known error case because *nobody* can think of a reason you'd catch that"

Larry just gave one: better error handling.

Error handling is one of the very well done part of PSR-6 - far better that eg how Doctrine Cache behaves. (eg when caching unserialisable objects, PSR-6 tells what to do).

Excellent error handling is a *strong* argument to me and a requirement for a standard to me.

- It is useful for routers to fetch a controller from the container.

What "id" should the router "get()"?
 

- It is useful for writing adapters extending any containers.

- It is useful for composing containers.

Sure - this is a very very thin and specific use case though to me. Nothing as big as other accepted PSRs.

 

- It is useful for writing factories that do not depend on a particular container.

Same here: what "id" should such factories "get()"?

If this is left to the implementer, it means PSR11 doesn't solve anything for this use case (and the router one).

Also, I'd like to make a parallel with PSR-7 and PSR-15. PSR-7 is already useful by itself, but it will be immensely more useful when PSR-15 passes. It is the same with PSR-11. It will be the cornerstone of interoperable service providers. As soon as PSR-11 passes, we plan forming a working group on standardizing service providers (see https://github.com/container-interop/service-provider/ for more details). PSR-11 is a prerequisite.

This is running in circle to me - self referencing statements to me. PSR-15 does not exist, thus it can't justify PSR-11.

And looking at PSR-15, how does it solve the "what "id" should the service provider "get()" from the container?"

Last note, but not least: having the experience of implementing PSR-16 and PSR-6, I *really strongly* suggest that next PSRs, this one included, always come with a reference battle-ready test suite.

We now have one that we share with php-cache on Symfony. And if we've had it before the acceptance, I would eg have been more insistent on the need for the "array" type hint on "*Multiple" methods there, because "array|Traversable" is NOT easily resolved with iterator_to_array. That's what I thought on the first implementation I submitted here. Then tests reminded me that iterators can be non-rewindable - and that keys can be of arbitrary types. And that it's a nightmare to deal with - thus PSR-16 is NOT going to be easy to correctly implemented - which is bad for a standard IMHO.

So, back to PSR11, it'd be great - if not mandatory to me - to have a reference test suite that provides actual side effects and verifiable outcomes for every single sentence in the RFC. That would clearly help implementers understand the sometimes too abstracts words - as spotted before.

Cheers,

Nicolas

[Fabien Potencier]

On 1/12/17 15:08, Larry Garfield wrote:
> Late in the week, I know, but this is when I had time. :-(
>

> Two:
>
> I understand the reasons why the delegate lookup was removed outright,
> and I do not dispute them. However, the net result is that we have an
> interface that is almost trivial, on a topic that is of questionable
> appropriateness in the first place. (I still question the validity of a
> container interface at all.) That is, with that removed, what's left to
> standardize? get() and has() are barely an specification.
>

I think we, as a group, underestimate the consequences of Larry's
concern number 2 (above).

Of course, having a specification with just a get() and a has() methods
makes it really easy to implement; and in fact, doing so in Symfony is
trivial (see https://github.com/symfony/symfony/pull/21265).

But is it really enough for interoperability? I doubt it. I won't even
talk about configuration as it has been debated in the past. Containers
have more features and they are using it. So, internally, each container
is going to still use their interfaces and not the simple PSR one. So,
no interoperability here. There is also the discoverability issue (no
set, so what about id discoverability), but this has already been
discussed as well. The delegate lookup feature would have been a good
selling point for this PSR (even if personally I disagree with it
usefulness), but it's out now.

I do understand that the group spent a lot of time trying to figure out
something that works for most projects, and I really appreciate their
work, but **can we admit that the result is deceptive and not useful
enough to standardize?**

just my 2cts of course,
Fabien

[Máté Kocsis]

As someone who finds ContainerIterface very useful, I'd like to respond to Nicolas' question:

What "id" should the router "get()"?

The router should "get()" the "id" which is configured for the current route.

At least this is what I do in my application framework with FastRoute:

- User configures actions as callables in the router (https://github.com/woohoolabs/harmony#define-your-routes)

- Router finds the current route and stores the appropriate callable

- Dispatcher fetches the previous callable by which it instantiates a controller (if possible) then invokes the action (https://github.com/woohoolabs/harmony/blob/master/src/Middleware/DispatcherMiddleware.php#L49)

Without ContainerInterface, I should have used an Adapter (e.g. the Acclimate package) or I could have hardwired a specific container into my framework to achieve so (I would have hated both solutions).

I know that you don't like the fact that Container "id"-s can't be really reckoned upon, but in many cases (like in my example) it is possible to "shift the responsibility" to the end-user to supply the appropriate "id"-s for the container so that containers don't have to know anything about the "id"-s.

I find PSR-11 fascinating because it is a very simple and elegant common ground for use-cases where knowing the "id"-s by the container is not needed. My libraries are very simplicistic so maybe that's why they usually fall in this category. I can admit however that it is very likely that this is not always the case: then PSR-11 is truly not very useful for those projects. But I do think that the majority of libraries will hugely profit from PSR-11 because they can become loosely coupled with containers - but yet easily connected to them.

Cheers,

Máté

[Nicolas Grekas]

Thanks for your answer Máté

Without ContainerInterface, I should have used an Adapter (e.g. the Acclimate package) or I could have hardwired a specific container into my framework to achieve so (I would have hated both solutions).

It looks like the only feature of PSR11 that this is using is the "get" method.

No "has", and no exception-related logic.

That basically means a simple closure would do the job equally well, isn't?

[Alessandro Lai]

I don't think that the  fact that PSR-11 is limited to a get is a reason to worry. In my opinion we should see this standard as a stepping stone for everything about containers.

Basically, I foresee in the feature various other container-related PSR, and PSR-11 will just be a prerequisite, like PSR-0 has been for PSR-4. Please, don't let the limited scope of this PSR be a disadvantage, because IMHO it's the opposite. 

[Matthew Weier O'Phinney]

Hi, all!

I'm going to respond to a couple different posts in this thread at
once, quoting the bits I want to respond to.

On 2017-01-13 Nicolas Grekas wrote:
>
> the exception part is really where things start to look like a zoo. Because we have many animals there in the "Exception" namespace, and mapping them to PSR11's NotFoundExceptionInterface is going to be non trivial and never perfect I'd say.
>
> If you read the thread, you'll see that this sentence "Exceptions directly thrown by the container" is unclear for now - what does "directly" mean?

It means exceptions that the container throws itself, vs. an exception thrown
elsewhere, such as by a factory it invokes, or PHP itself (for type errors).

The container will generally only need to throw exceptions directly for a
handful of issues:

- If a dependent service is missing.
- Naming conflicts (unlikely, but can happen).
- Inability to resolve an argument; as an example, in reflection-based
containers, when encountering required arguments without typing or with scalar
typehints.

Essentially, these are a form of RuntimeException; you're attempting to create a
service, but cannot, due to problems in container configuration.

> > the NotFoundExceptionInterface triggered by the inner get call SHOULD NOT bubble out.
>
> Fortunately enough, this is only a "SHOULD NOT", so we can do as we want and still be compliant. Because I agree with Larry here, the wording is so abstract that in practice it hardly helps. What does missing mean? Eg does a brokenly configured first level service qualify as such?
> Reasoning from the inside of the container, you may say no.
> But reasoning from the outside, the user might expect a yes, so that eg a chained-container can continue with the next container in the chain (provided NotFoundExceptionInterface VS ContainerExceptionInterface could be the trigger to decide if the chain should break or continue? what is it for otherwise? doesn't that concept duplicate the "has()" method?)

This sounds like using exceptions for control flow, which is often a
questionable design choice.

I agree with Nicolas here, however, in that I'd likely allow the
NotFoundExceptionInterface to bubble out in this case, not for control flow, but
to make it clear to the end user that a dependency is missing from the
container.

> > "Don't standardize an *exception interface* for a known error case because *nobody* can think of a reason you'd catch that"
>
> Larry just gave one: better error handling.
> Error handling is one of the very well done part of PSR-6 - far better that eg how Doctrine Cache behaves. (eg when caching unserialisable objects, PSR-6 tells what to do).
> Excellent error handling is a *strong* argument to me and a requirement for a standard to me.

I agree... except that no concrete examples have been provided of when such an
exception should be thrown, nor when it would be useful to catch it. If you take
the above example I note of having a factory raise a
`NotFoundExceptionInterface` instance in the case that a dependency it requires
is missing, there's no need for an additional exception.

Turning now to these:

On 2017-01-13 Larry Garfield wrote:
>
> That is, with that [delegate lookup] removed, what's left to standardize? get() and has() are barely an specification.

and:

On 2017-01-13 Fabien Potencier wrote:
>
> Of course, having a specification with just a get() and a has() methods makes it really easy to implement; and in fact, doing so in Symfony is trivial (see https://github.com/symfony/symfony/pull/21265).
>
> But is it really enough for interoperability? I doubt it.

Máté note an example where he composed a ContainerInterface in a router, and how
that allowed him to have a container-agnostic router. Nicolas dismissed this,
saying that because it does not use `has()`, it could very well have simply used
a callable.

I'm going to provide another example. It's similar to Máté's: the Expressive
routing and dispatch system.

We *do* use `has()`; if that returns false, we're able then to raise an
application exception indicating that the middleware to which the request routed
is invalid and/or missing. Otherwise, we pull it using `get()`, and, if that
operation raises an exception, we can report something completely different.

However, container-interop provided an important piece outside of the defined
methods, and *even despite the lack of a configuration specification*. How?

Factories.

We can write factories that only hint on container-interop. We are then able to
provide common configuration that we then adapt to each container type we
currently support. This work was trivial, but allows developers to select the
container that best suits their purposes. It even allows them to select
something different — e.g., Disco — and adapt the configuration for their own
needs.

But the factories work the same *regardless of the container used*.

This allowed us to make the design decision that the application and router only
care about a ContainerInterface.

So, yes, the standard as it is currently *does* provide interoperability. It's
not a nebulous thing at all; I and many others *have already built on it*;
that's why there's been a working group. It's already field-tested. Whether or
not each of you ultimately feel it's a fit for your own projects is your own
choice, but I don't think you can make a blanket declaration that it's worthless
when others have been actively building with it for quite some time now.

Finally, to each of those commenting JUST TODAY: I get it, we're all busy.
However, this is the SECOND review period we've had, and you've waited until
literally the last day of that to raise objections. Please, all of us, try to be
more diligent; these are conversations we should have had weeks or months ago,
not this late in the process.

[Máté Kocsis]

That basically means a simple closure would do the job equally well, isn't?

Yes, for such a basic use-case like mine, a Clousure would be also sufficient but type-hinting would be sacrified when doing so (I mean it is more difficult to document the Closure's signature).

Furthermore, if a container offers different methods for fetching Singleton and Prototype entries then it is not obvious which method to use in the Clousure (controllers should be Singletons for sure, but let's generalize the problem further than the dispatcher example). Via a common interface like ContainerInterface which defines a single method for fetching, objects can be consumed regardless if they are Singleton or Prototype: consumers only have to trust the developer if configuration is appropriate.

[Daniel Plainview]

> Larry just gave one: better error handling.

You can use this argument for any exception.

Furthermore, I don't think it is correct to handle such "error" at this stage.

You don't need to catch *exactly* this exception (MissingDependency/Misconfigured), because you can't auto-fix it at runtime.

Probably, explanation of NotFoundException should be more clear, but it isn't a reason to standardize new exception.

[Jason Judge]

PR 866 https://github.com/php-fig/fig-standards/pull/866 fixes this formatting issue. It doesn't fix the obvious bug in github causing it, but it uses the alternate markdown syntax that avoids it.

-- Jason

On Thursday, 12 January 2017 23:08:10 UTC, Larry Garfield wrote:

...

[Jason Judge]

The parser does not look far enough ahead. It sees the text of the heading that follows it, and thinks that text is a part of the list (list items with blank lines in are a valid syntax for lists with multi-paragraph contents). It does not spot the "----------" that follows the heading text which turns it into an actual heading. The alternate heading syntax (##) allows the list parsing to spot that though, as it does not need to look any further than the heading line. I've submitted a PR for this.

-- Jason

On Friday, 13 January 2017 10:46:29 UTC, David Négrier wrote:

...

[David Négrier]

> > the NotFoundExceptionInterface triggered by the inner get call SHOULD NOT bubble out.
>
> Fortunately enough, this is only a "SHOULD NOT", so we can do as we want and still be compliant. Because I agree with Larry here, the wording is so abstract that in practice it hardly helps. What does missing mean? Eg does a brokenly configured first level service qualify as such?
> Reasoning from the inside of the container, you may say no.
> But reasoning from the outside, the user might expect a yes, so that eg a chained-container can continue with the next container in the chain (provided NotFoundExceptionInterface VS ContainerExceptionInterface could be the trigger to decide if the chain should break or continue? what is it for otherwise? doesn't that concept duplicate the "has()" method?)

This sounds like using exceptions for control flow, which is often a
questionable design choice.

I agree with Nicolas here, however, in that I'd likely allow the
NotFoundExceptionInterface to bubble out in this case, not for control flow, but
to make it clear to the end user that a dependency is missing from the
container.

Ok, let me just explain how we decided to handle exceptions this way. This might help the debate here.

Our first idea was that the NotFoundExceptionInterface should have a `getMissingIdentifier()` method.

Let's pretend you are using a PSR-11 container. In my case, I was building a UI that was letting you explore a container. The user types an entry name, the UI displays informations about the returned service.

If I catch a NotFoundExceptionInterface, it has a very different meaning if the NotFoundException is thrown by my first `get` call or an inner get call. In one case, the user typed a buggy entry name, in the other case, the container is misconfigured.

So my code was looking like this:

try {
    $service = $container->get($id);
} catch (NotFoundExceptionInterface $e) {
    if ($id === $e->getMissingIdentifier()) {
        // Display an error message to the user saying the service does not exist
    } else {
        // Oops, I should not have caught that in the first place, I don't know what to do with this, let's rethrow this.
        throw $e;
    }
}

When someone catches the "NotFoundException", the "if" statement inside the "catch" was almost systematic.

This is because we actually have 2 different exceptions. One is "the user provided an invalid identifier", and the other is: "the container configuration somewhere has an error, with probably an invalid identifier stored".

So we decided that those 2 exception cases should be treated differently.

By restricting the scope of the NotFoundExceptionInterface to the first outermost `get` call, one user can write:

try {
    $service = $container->get($id);
} catch (NotFoundExceptionInterface $e) {
    // Display an error message to the user saying the service does not exist
    // No need to bother checking that the caught exceptions relate to $id, this is sure.
}

Now, I understand this can be discussed in many ways.

For instance, one could argue that my example is buggy, because I'm using exceptions for control flow. If I'm dealing with user input, I should first use "has" (and therefore, I'm sure the entry does exist, so if a "XxxNotFoundException" is triggered, it has to be a "missing dependency exception".

I don't think there is "one and only one" truth regarding the definition of exceptions. It really depends on where one puts the cursor, and the FIG has never defined clearly which exceptions should be part of a PSR and which should be out of it.

@Nicolas, @Mwop, if you think the exception handling of PSR-11 can be improved, could you describe precisely what you would change, why and how? Is this a wording issue or is this a design issue? Also, is this completely blocking you from implementing PSR-11? And is this a subject that could change a -1 to a +1 vote (that question is for Fabien and Larry :) )

As you might guess, I'm pretty happy with the exceptions the way they are defined currently.

Yet, I'm really happy to discuss this issue if you think it is worthwhile, but we have a strong deadline. If we change the way exceptions are handled, we must do so before Tuesday, 17th because it is the last possible date for a PSR to enter (again) a review phase, so please, give me your comments as quickly as possible.

++
David.

[Larry Garfield]

On 01/14/2017 11:37 AM, David Négrier wrote:

*snip*

So my code was looking like this:

try {
    $service = $container->get($id);
} catch (NotFoundExceptionInterface $e) {
    if ($id === $e->getMissingIdentifier()) {
        // Display an error message to the user saying the service does not exist
    } else {
        // Oops, I should not have caught that in the first place, I don't know what to do with this, let's rethrow this.
        throw $e;
    }
}

When someone catches the "NotFoundException", the "if" statement inside the "catch" was almost systematic.

This is because we actually have 2 different exceptions. One is "the user provided an invalid identifier", and the other is: "the container configuration somewhere has an error, with probably an invalid identifier stored".

So we decided that those 2 exception cases should be treated differently.

I agree entirely on this point!  These are two separate error conditions, and thus should have 2 separate exceptions.

By restricting the scope of the NotFoundExceptionInterface to the first outermost `get` call, one user can write:

try {
    $service = $container->get($id);
} catch (NotFoundExceptionInterface $e) {
    // Display an error message to the user saying the service does not exist
    // No need to bother checking that the caught exceptions relate to $id, this is sure.
}

Now, I understand this can be discussed in many ways.

For instance, one could argue that my example is buggy, because I'm using exceptions for control flow. If I'm dealing with user input, I should first use "has" (and therefore, I'm sure the entry does exist, so if a "XxxNotFoundException" is triggered, it has to be a "missing dependency exception".

I don't think there is "one and only one" truth regarding the definition of exceptions. It really depends on where one puts the cursor, and the FIG has never defined clearly which exceptions should be part of a PSR and which should be out of it.

My "cursor" is that defining what code should do on the not-happy path is at least as important if not moreso than what happens on the happy path, especially for standards.  The history of HTML and CSS should have seared that into our brains by now. :-)

(Most of the incompatibility of different HTML engines in the bad old days wasn't in how they handled perfect markup but in how they handled the million ways in which markup could be broken.  The HTML5 spec is so enormous precisely because it tries to retcon the many different ways of handling invalid code into a coherent standard.)

My main issue is that the current text says "In situation A, do X.  In situation B, do any capital letter that is not A."  That is a construction I simply cannot get behind in any specification of any sort, as it is simply too vague and begs for inconsistent error handling.  (Eg, every browser handling a missing </div> tag differently.)

I have offered two alternatives that would be acceptable: In situation B, throw MisconfiguredServiceException or MissingDependencyException.  Either one would resolve the issue with the spec's error handling.  (Based on earlier discussion I agree that MissingDependency is more specific and thus a better approach.)  If neither of those are acceptable to you, please offer your own alternative that resolves the incomplete error handling.

--Larry Garfield

[Daniel Plainview]

> My main issue is that the current text says "In situation A, do X.  In situation B, do any capital letter that is not A."  That is a construction I simply cannot get behind in any specification of any sort, as it is simply too vague and begs for inconsistent error handling

There is no such situation like B from consumer point of view, thus you don't need to describe what to do in this case.

You can rephrase text, e.g. "Throw NotFoundException when and only when the requested service is not found",

and define what "requested service" is.

Probably, it makes sense to add example of right and wrong pseudo implementations to the spec.

[David Négrier]

Hey people,

This might sound as a surprise, but thanks to Nicolas' and Matthew's comments, I came up with yet another solution that was never really discussed.

PR (WIP) is here: https://github.com/php-fig/fig-standards/pull/869

Nicolas and Matthew pointed a few things that were never said before.

Namely:

• The NotFoundExceptionInterface duplicates the concept of the has method (in Nicolas post)
• Using exceptions for control flow is often a questionable design choice (in Matthew post)

Based on these 2 assertions (I agree with both), we should question the usefulness of NotFoundExceptionInterface as it is defined today.

We spent a lot of time to make sure that containers implementing PSR-11 are differentiating the NotFoundExceptionInterface from a "missing dependency exception" (that should exist in the container even if it is not standardized). This involves that implementing containers MUST catch the NotFoundExceptionInterface and rethrow another exception. So we are forcing an "implementation detail" into implementors.

And I've come to realize that contrary to what I believed, we absolutely don't need that.

The change I'm proposing is dead simple: let's drop any distinction between "entry not found" and "dependency missing". We don't need this because users of containers can anyway figure that out.

If a user of a container wants to make the distinction between "not found" and "missing dependency", he can simply perform a call to has before calling get:

if (!$container->has($id)) {
    // The requested instance does not exist
    // Let's do some stuff like display an error message
    return;
}
try {
    $entry = $container->get($id);
} catch (NotFoundExceptionInterface $e) {
    // Since the requested entry DOES exist, a NotFoundExceptionInterface means that a dependency is missing!
}

Advantages:

• it is dead simple
• we don't dictate that NotFoundExceptionInterface must not bubble up (no forced catch and rethrow, which feeled a bit flacky)
• containers can do whatever they want:
  • either let NotFoundExceptionInterface bubble up
  • or catch the NotFoundExceptionInterface and wrap it in a "MissingDependencyException" of their own that must itself implement the NotFoundExceptionInterface

Inconvenient:

• slight incompatibility with container-interop, since in container-interop, the "MissingDependencyException" was not supposed to implement the NotFoundExceptionInterface (this is actually not clearly stated in the container-interop documentation)

The more I think about it, the more I'm seduced by this solution. It is simple, it leaves complete freedom to implementors, and container users can still figure out if the exception they get is a "not found" or a "missing dependency" with a simple has call.

It also enforces best practices as users that want to check if an entry exists will use has instead of NotFoundExceptionInterface (the exception can no more be used for control flow)

Note: I haven't had a chance to speak with Matthieu, the other editor, about it yet since deadlines are tight, I'm sharing it right now. Matthieu, if you think this is foolish, we can cancel that.

As you can see, the PR makes PSR-11 even more simple, which is generally a good sign: https://github.com/php-fig/fig-standards/pull/869

What do you guys think?

++

David.

[Nicolas Grekas]

Hi David,

let's drop any distinction between "entry not found" and "dependency missing". We don't need this because users of containers can anyway figure that out.

I'd like to propose the opposite alternative. That's no a rebuttal of yours and I sincerely thank you for being so open to comments.
Let's see:
My naïve question was "doesn't that concept [NotFoundExceptionInterface] duplicate the "has()" method?"

Your new proposal is to answer a strict "no" to this question, thus remove the exception all together.

What about answering it with a strict "yes"?

What I mean is to keep NotFoundExceptionInterface and define it a to be thrown by "get" only and only if would have returned false.

I think that would fix Larry's comments, and more importantly, fix any ambiguity and the semantic of this exception.

So, to sum up, error handling in PSR11 could look like:

- "has" never throws anything but instanceof \Error - it only returns true/false

- "get" throws NotFoundExceptionInterface whenever "has" would have returned false - it "directly" throws ContainerExceptionInterface - and is allowed to let anything else bubble if thrown by the factories/initializers it calls.

 

It also enforces best practices as users that want to check if an entry exists will use has instead of NotFoundExceptionInterface (the exception can no more be used for control flow)

This is the main difference with your proposal: NotFoundExceptionInterface could be used for flow control. But since doing so would be strictly equivalent to call "has", we could even add in the PSR a SHOULD NOT - if worth it.

Does it make sense?

Nicolas

[matt...@mnapoli.fr]

Hi Nicolas,

Thanks for the effort to move this forward.

Regarding you last suggestion I'm having trouble seeing the difference with the current version of the spec, could you pinpoint it more explicitly?

Thanks

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/I1a2Xzv9wN8/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/CAOWwgpmQZnbvCfLkE58mDhBT3NHCWqdkPabPEsJRkezTHZm-AQ%40mail.gmail.com.

[Nicolas Grekas]

Regarding you last suggestion I'm having trouble seeing the difference with the current version of the spec, could you pinpoint it more explicitly?

There are two differences:

The most important one is that the *wording* of the PSR need to be as clear as possible. Finding a clear way to express things, so that the PSR is self explanatory enough, is a must (the answers on the ML will be lost in history.)

The second one is a really technical consequence I didn't realize when writing the previous comment:

It makes this statements useless (which is a good thing since that's the one we're worried about):

> A call to get can trigger additional calls to get (to fetch the dependencies). If one of those dependencies is missing, the NotFoundExceptionInterface triggered by the inner get call SHOULD NOT bubble out. Instead, it should be wrapped in an exception implementing the ContainerExceptionInterface that does not implement the NotFoundExceptionInterface.

If one wants to be able to really differentiate these situations, one can already do:

just check "has" before "get". If, after doing so, "get" throws a NotFoundExceptionInterface, then it's a second level missing dep. Isn't it?

Btw, don't we miss an "InvalidArgumentException", to be thrown when the arg passed to has/get is not a string?

Nicolas

[Daniel Plainview]

> Btw, don't we miss an "InvalidArgumentException", to be thrown when the arg passed to has/get is not a string?

It has as much value as documenting `@throws \Error` (compare it with scalar type hints).

[Alessandro Lai]

I really like this proposal y Nikolas, especially the part about the SHOULD NOT use the exception for control flow...

[David Négrier]

The second one is a really technical consequence I didn't realize when writing the previous comment:

It makes this statements useless (which is a good thing since that's the one we're worried about):

> A call to get can trigger additional calls to get (to fetch the dependencies). If one of those dependencies is missing, the NotFoundExceptionInterface triggered by the inner get call SHOULD NOT bubble out. Instead, it should be wrapped in an exception implementing the ContainerExceptionInterface that does not implement the NotFoundExceptionInterface.

If one wants to be able to really differentiate these situations, one can already do:

just check "has" before "get". If, after doing so, "get" throws a NotFoundExceptionInterface, then it's a second level missing dep. Isn't it?

This.

This is really what I realized yesterday and what I meant to say in my previous post.

Have a look at the PR I wrote: https://github.com/php-fig/fig-standards/pull/869/files

That's exactly what you are proposing in your comment (I'm removing the complete statement you are worried about). Are we on the same line here or is there something else you would change? Reading your comments, I believe we have exactly the same idea.

++
David.

[David Négrier]

Ok, we have 3 options, and one day to decide which to choose.

In order to ease the choice, I opened 3 PRs on Github.

I'll ask everyone involved to give feedback on the 3 PRs.

Let me present them briefly.

PR 1: Status Quo: NotFoundExceptionInterface CANNOT bubble up, but we do not standardize MissingDependencyException
Link: https://github.com/php-fig/fig-standards/pull/870/files

This PR simply improves the current wording. It is easier to understand but is strictly equivalent to PSR-11 as it is today.


PR 2: NotFoundExceptionInterface CAN bubble up
Link: https://github.com/php-fig/fig-standards/pull/869/files

This PR let's NotFoundExceptionInterface bubble up. So a `NotFoundExceptionInterface` means either "entry not found" OR "missing dependency". The user can make the difference between the 2 using the `has` method.

PR 3: NotFoundExceptionInterface CANNOT bubble up, but we add a MissingDependencyException
Link: https://github.com/php-fig/fig-standards/pull/871/files

This PR adds an explicit MissingDependencyExceptionInterface.


Could I ask each of you to have a brief look at these PR and to tell us how you like those?
Also, for the voting member, please indicate if one of these PRs would change your vote from -1 to +1 (or the other way around)


Ok, so I'm opening this "flash poll" so I'll start:

PR 1: Status Quo https://github.com/php-fig/fig-standards/pull/870/files

++: I'm happy with the rewording, thanks to Nicolas comments.


PR 2: NotFoundExceptionInterface CAN bubble up https://github.com/php-fig/fig-standards/pull/869/files

++: I'm equally happy with this proposal. Looking at most container-interop implementations (for instance acclimate), they failed to propertly prevent NotFoundExceptionInterface from bubbling up. So they are actually already implementing this PR (by accident).

PR 3: NotFoundExceptionInterface CANNOT bubble up, but we add a MissingDependencyException https://github.com/php-fig/fig-standards/pull/871/files

-: I could perfectly live with an explicit MissingDependencyExceptionInterface (it cannot break anything), but as explained earlier, I find no good reason to catch it, and it prevents us from migration Container-interop to PSR-11 automatically. So if possible, I'd like to avoid that (but this is not a show stopper for me and I'm ok to go that way if needed)

Please answer this as soon as possible (since if we do any big changes to the PSR, tomorrow is the last day we can put PSR-11 for review).

++
David.

[Alessandro Pellizzari]

On 13/01/2017 16:48, Fabien Potencier wrote:

> I do understand that the group spent a lot of time trying to figure out
> something that works for most projects, and I really appreciate their
> work, but **can we admit that the result is deceptive and not useful
> enough to standardize?**

I disagree.

Before this PSR, most containers relied on __get magic methods, with no
rules on keys or errors.

An interface is not just the function name. it's mostly its signature:
what it accepts, what it returns, what errors it can generate/throw.

This PSR makes containers interoperable, because a well-made container
knows other containers will have the same interface (and exceptions), so
if it wants to implement delegation, it can do it easily.

It makes libraries interoperable, because they abstract over the container.

It makes applications interoperable, because you can just change the
container and its configuration in the bootstrap and the rest of the app
continues working without changes.

It's simple, yes, but that's a plus, not a defect.

A way to re-introduce delegation into the specs would be, for example,
to define the exceptions thrown when delegation fails.

It's quite logical that if a container adheres to the specs just by
wrapping its external interface, it loses the advantages of delegation.
You need to change the inner working to take advantage of it, but with
this PSR you can do it.

I still haven't made up my mind around the cascade gets exceptions, but
I think this PSR is quite important as it is.

Bye.

[Matthieu Napoli]

Thanks David for the summary and those PRs.

PR 1 sounds like the best solution to me.

I can live with PR 2 or PR 3 if it helps the adoption of this PSR, I think PSR-11 would be better without these but it's my opinion and I see that as a minor detail.

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/I1a2Xzv9wN8/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/398f3c10-6e15-4248-ae89-2193968cffa7%40googlegroups.com.

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

[Larry Garfield]

On 01/16/2017 04:58 AM, David Négrier wrote:

Ok, we have 3 options, and one day to decide which to choose.

In order to ease the choice, I opened 3 PRs on Github.

I'll ask everyone involved to give feedback on the 3 PRs.

Let me present them briefly.

PR 1: Status Quo: NotFoundExceptionInterface CANNOT bubble up, but we do not standardize MissingDependencyException
Link: https://github.com/php-fig/fig-standards/pull/870/files

This PR simply improves the current wording. It is easier to understand but is strictly equivalent to PSR-11 as it is today.


PR 2: NotFoundExceptionInterface CAN bubble up
Link: https://github.com/php-fig/fig-standards/pull/869/files

This PR let's NotFoundExceptionInterface bubble up. So a `NotFoundExceptionInterface` means either "entry not found" OR "missing dependency". The user can make the difference between the 2 using the `has` method.

PR 3: NotFoundExceptionInterface CANNOT bubble up, but we add a MissingDependencyException
Link: https://github.com/php-fig/fig-standards/pull/871/files

This PR adds an explicit MissingDependencyExceptionInterface.


Could I ask each of you to have a brief look at these PR and to tell us how you like those?
Also, for the voting member, please indicate if one of these PRs would change your vote from -1 to +1 (or the other way around)

Unsurprisingly I'd favor PR 3: Explicit exception.

I'll be honest that I'm still very confused why there's so much resistance from the Editors on using exception types for what they're supposed to be used for.  Different error condition, different exception.  In 20 years of writing code I cannot recall ever saying "damn, this error message is too precise and specific".  I have said the opposite more times than I can count.

PR 1 would resolve the awkward wording, although I do feel it is the inferior approach as it is less precise and provides less contextual information for callers.  (That we don't at the moment know when a caller would want that extra information is, IMO, irrelevant as it's so easy to provide.)

PR 2 actively works to make using the spec harder, and I cannot get behind that at all.

I am admittedly unlikely to vote +1 on the spec regardless, but should it pass anyway I feel it should still be as good (or least bad) as possible, and that is PR 3.

--Larry Garfield

[Chuck Burgess]

I'm good with #3, as it looks like a useful addition that should alleviate some concerns.

--

You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+unsubscribe@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/398f3c10-6e15-4248-ae89-2193968cffa7%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Daniel Plainview]

> Different error condition, different exception.  In 20 years of writing code I cannot recall ever saying "damn, this error message is too precise and specific".

@Larry nobody says the opposite, we all agree on this one.

Of course, it must be different exception with nice and precise class name and descriptive exception message

(we want to have straightforward messages in our error log, but in case of logging, we catch *all* exceptions).

We just say that it's out of the spec.

Just like CircularDependencyException.

There is no this exception in the spec.

And I really don't see why you make MissingDependencyException more special over CircularDependencyException.

They are both very important, but they don't bring value in this context.

P.S. I really don't want to offend anyone, I'm just very curious to understand opinions on this topic.

[Matthew Weier O'Phinney]

From a consumer standpoint, I can see why this would be useful. If PR
1 is used, we end up with a call to get() raising a
NotFoundExceptionInterface instance if either of the following
conditions is true:

- The container does not have the "knowledge" necessary to create an
instance of $id.
- One of the dependencies of $id does not exist.

Being able to differentiate *may* be of use at that point. However,
from experience, if I see a NotFoundExceptionInterface, the only thing
of use to me as a consumer is determining *which* service is missing;
I do not care if it is a *dependency* of another service, just that
it's needed in my object graph, and I didn't provide relevant
information to create it.

From an implementer's standpoint, PR 3 is a nightmare, as it
*requires* that any attempt to retrieve a dependency is wrapped in a
try/catch block internally. As an example:

public function get($id)
{
if (isset($this->map[$id])) {
return $this->map[$id];
}

try {
$instance = $this->create($id);
return $instance;
} catch (NotFoundExceptionInterface $e) {
throw new MissingDependencyException(sprintf(
'Could not create "%s" due to a missing dependency',
$id
), $e->getCode(), $e);
}
}

Essentially, the try/catch block MUST be present. This is specifying
_internal implementation_, and, of more concern, specifying an
implementation based on try/catch which has significant performance
issues.

If we go with PR 1, the above can be simplified, based on the needs of
the implementation:

public function get($id)
{
if (isset($this->map[$id])) {
return $this->map[$id];
}

return $this->create($id);
}

Any NotFoundExceptionInterface raised now bubbles up, and I can see
what is missing and blocking execution. Alternately, my implementation
*could* still wrap in a try/catch block and provide a
MissingDependencyException that acts as a subtype of
NotFoundExceptionInterface; the spec would not disallow it, but also
wouldn't force this implementation detail.

As such, I'm in favor of option 1.

[Daniel Plainview]

> If PR 
> 1 is used, we end up with a call to get() raising a 
> NotFoundExceptionInterface instance if either of the following 
> conditions is true: 

> - One of the dependencies of $id does not exist. 

PR 1 doesn't bubble out NotFoundException in this case.

PR 2 acts like you described, though.

[Matthieu Napoli]

Larry,

Unsurprisingly I'd favor PR 3: Explicit exception.

I'll be honest that I'm still very confused why there's so much resistance from the Editors on using exception types for what they're supposed to be used for.  Different error condition, different exception.  In 20 years of writing code I cannot recall ever saying "damn, this error message is too precise and specific".  I have said the opposite more times than I can count.

David works on his personal time to implement something that you want, we both say that we are ready to accept that change if you think this is mandatory for you to accept the spec, I don't see how you can call that "resistance" but maybe that's just a wording issue and I shouldn't take too much notice of that.

Also I just want to point out that of course there should be very precise and explicit exceptions, we all agree on that. It's just that we don't believe the interface (as such, the spec) makes sense because you wouldn't need to catch it. Implementors *should* make very explicit exceptions. Anyway, again this is just my opinion and I understand you may not share it, I just want to make sure we are talking about the same thing.

PR 1 would resolve the awkward wording, although I do feel it is the inferior approach as it is less precise and provides less contextual information for callers.  (That we don't at the moment know when a caller would want that extra information is, IMO, irrelevant as it's so easy to provide.)

PR 2 actively works to make using the spec harder, and I cannot get behind that at all.

I am admittedly unlikely to vote +1 on the spec regardless, but should it pass anyway I feel it should still be as good (or least bad) as possible, and that is PR 3.

If I understand correctly you might vote -1 because:

> That is, with that [the dependency lookup feature] removed, what's left to standardize?  get() and has() are barely an specification. 

What do you think of the fact that some frameworks (Slim, Zend Expressive, Silly…) or some libraries (Behat…) are able to integrate with any container thanks to ContainerInterface? Do you disagree it justifies the existence of PSR-11?

Matthieu

[Matthew Weier O'Phinney]

On Mon, Jan 16, 2017 at 10:01 AM, Daniel Plainview
<daniel...@gmail.com> wrote:
>> If PR
>> 1 is used, we end up with a call to get() raising a
>> NotFoundExceptionInterface instance if either of the following
>> conditions is true:
>> - One of the dependencies of $id does not exist.
>
> PR 1 doesn't bubble out NotFoundException in this case.
> PR 2 acts like you described, though.

I got confused between the options and the order in which the PRs were
submitted, and David Négrier has pointed out the same to me elsewhere

Yes, I meant option 2, allowing NotFoundExceptionInterface to bubble out.

> --
> You received this message because you are subscribed to the Google Groups

> "PHP Framework Interoperability Group" group.

> To unsubscribe from this group and stop receiving emails from it, 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/bf9625dc-6c52-41f5-96d6-e254c7ee8ea9%40googlegroups.com.
>
> For more options, visit https://groups.google.com/d/optout.

[Nicolas Grekas]

No surprise, I 100% agree with Matthew's analysis. Option 2 also for me.

[David Négrier]

First of all, thanks a lot for being so quick to comment this issue.

Let's summarize the expressed opinions.

(I'll try to put a note from -- to ++ based on your textual feedback, excuse me if I'm not 100% accurate).


------------------------------|------|------|-------
Name                          | PR 1 | PR 2 | PR 3
------------------------------|------|------|-------
Chuck Burgess                 |  --  |  --  |  ++
Daniel Plainview              |  ??  |  ??  |  --
David Négrier                 |  ++  |  ++  |  -
Larry Garfield                |  --  |  --  |  ++
Matthieu Napoli               |  ++  |  +   |  +
Matthew Weier O'Phinney       |  -   |  ++  |  -
Nicolas Grekas                |  ??  |  ++  |  ??
@unkind                       |  ??  |  -   |  ??


Sum of (totally not scientific) results so far:

PR 1: +4/-5
PR 2: +7/-5
PR 3: +5/-4

So overall, it's a debated idea. I'm sure by tweaking a bit the numbers, we could get whatever result we want.

What I find interesting however, is that both Matthew Weier O'Phinney (Zend Framework) and Nicolas Grekas (Symfony) agree on this. And since they are representing 2 major DI containers, I guess we should listen to them carefully.

++
David

[Larry Garfield]

On 01/16/2017 10:15 AM, Matthieu Napoli wrote:

Larry,

Unsurprisingly I'd favor PR 3: Explicit exception.

I'll be honest that I'm still very confused why there's so much resistance from the Editors on using exception types for what they're supposed to be used for.  Different error condition, different exception.  In 20 years of writing code I cannot recall ever saying "damn, this error message is too precise and specific".  I have said the opposite more times than I can count.

David works on his personal time to implement something that you want, we both say that we are ready to accept that change if you think this is mandatory for you to accept the spec, I don't see how you can call that "resistance" but maybe that's just a wording issue and I shouldn't take too much notice of that.

Also I just want to point out that of course there should be very precise and explicit exceptions, we all agree on that. It's just that we don't believe the interface (as such, the spec) makes sense because you wouldn't need to catch it. Implementors *should* make very explicit exceptions. Anyway, again this is just my opinion and I understand you may not share it, I just want to make sure we are talking about the same thing.

I very much appreciate that we're all working on (mostly) volunteer time.  The resistance to which I refer is the past several weeks, in which the main refrain has been "we don't want to specify another exception."  It's the position of "yes it's a different error case but we don't want to specify it" that I find befuddling.  History has shown that imprecise definition of the not-happy path can be a major failing of a spec.  I'm not aware of any instance of error handling being harmfully over-specified.  Thus I am inclined to favor more precision over less when it comes to error handling.

As noted, PR 1 (being moot on the subject) would clear up my main issue regarding the "any capital letter but X" problem, despite still leaving most error handling undefined.  That is suboptimal.  (To the earlier question about specifying a CircularDependencyException as well, that's a complex enough case that I wouldn't insist on it but I wouldn't object if someone were to add it.)

Similarly, to Matthew's point about it "imposing an implementation detail", many of our specs do that in practice.  PSR-3 has only one meaningful way to implement all but one of its methods; we provided a trait for it for that reason.  PSR-7's use of immutability around what amounts to getters/setters for primitives (about half the methods on it) implies that there's very little flexibility in how much of it is implemented.  There's only one meaningful way to bridge PSR-16 to PSR-6.  Yet all of those were approved, because the awareness that there would be some boilerplate in any valid implementation didn't preclude there still being multiple valid and distinct implementations and there being value in standardization of the interface.

Certainly that can be a fuzzy area (there were people saying PSR-7 implied only one possible implementation entirely), and rational people can disagree on where that fuzzy line is.  For me, implying that everyone's get() implementation has a try-catch block in it warrants a ¯\_(ツ)_/¯.

PR 1 would resolve the awkward wording, although I do feel it is the inferior approach as it is less precise and provides less contextual information for callers.  (That we don't at the moment know when a caller would want that extra information is, IMO, irrelevant as it's so easy to provide.)

PR 2 actively works to make using the spec harder, and I cannot get behind that at all.

I am admittedly unlikely to vote +1 on the spec regardless, but should it pass anyway I feel it should still be as good (or least bad) as possible, and that is PR 3.

If I understand correctly you might vote -1 because:

> That is, with that [the dependency lookup feature] removed, what's left to standardize?  get() and has() are barely an specification. 

What do you think of the fact that some frameworks (Slim, Zend Expressive, Silly…) or some libraries (Behat…) are able to integrate with any container thanks to ContainerInterface? Do you disagree it justifies the existence of PSR-11?

I've been highly skeptical of PSR-11 since before it had a number. :-)  Factories along the line of Symfony's ControllerResolver (also used by Drupal) are the main use case and benefit I can see; in fact I could see myself using that were it to pass in one side library I have.  However, there are other ways to handle many such cases (which MWOP has in fact pointed out to me), and I am still concerned about the unintended side effect of encouraging Service Locator usage (despite the spec's recommendation to the contrary).  Without a major enabling of some other functionality (such as stitching together complex libraries without bridge modules to wire up a dozen different containers, which the delegation concept could have possibly helped with and I thought was its intent), that risk seems too great to me.

As above, reasonable people can disagree on that point, and that's fine.  But that's where I currently stand.

--Larry Garfield

[Larry Garfield]

Note: If you're using a single + to indicate a second choice, then I'd have a single + on PR 1.

--Larry Garfield

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, 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/6a519848-a45b-401e-a111-92c79cdb0860%40googlegroups.com.

[Daniel Plainview]

> Essentially, the try/catch block MUST be present. This is specifying 

> _internal implementation_, and, of more concern, specifying an 

> implementation based on try/catch which has significant performance 

> issues. 

Do I miss something or it can be implemented easier w/o try .. catch? Example:

public function get($id)

{

   if (!isset($this->services[$id])) {

       throw new NotFoundServiceException($id);

   }

    return $this->getService($id);

}

private function getService()

{

   // This method may recursively call itself

   // And this method never throws NotFoundServiceException

}

I think it's possible to do in Symfony at least w/o BC break.

On Monday, January 16, 2017 at 6:48:24 PM UTC+3, Matthew Weier O'Phinney wrote:

[Niklas Keller]

Yet, I'm really happy to discuss this issue if you think it is worthwhile, but we have a strong deadline. If we change the way exceptions are handled, we must do so before Tuesday, 17th because it is the last possible date for a PSR to enter (again) a review phase, so please, give me your comments as quickly as possible.

Why are we in a rush? https://groups.google.com/d/msg/php-fig/Rf1Xfgix3Cw/oDidLIGGBQAJ says:

> 31st December: Latest date for any PSRs to start approval votes under the old system

> 31st December: Final deadline for PSRs to have full working groups formed with a CC sponsor

> 1st January: All votes and actions take place under new system; full transition

> 31st January: Deadline for any PSRs which entered review before the 17th December to form a Working Group with a CC sponsor if they failed their approval vote/didn’t get to their approval vote in time

 

Where's a 17th deadline for January? In general, I don't think rushing specifications because of a strict dead line is a good thing.

[David Négrier]

> 31st December: Latest date for any PSRs to start approval votes under the old system

> 31st December: Final deadline for PSRs to have full working groups formed with a CC sponsor

> 1st January: All votes and actions take place under new system; full transition

> 31st January: Deadline for any PSRs which entered review before the 17th December to form a Working Group with a CC sponsor if they failed their approval vote/didn’t get to their approval vote in time

 

Where's a 17th deadline for January? In general, I don't think rushing specifications because of a strict dead line is a good thing.

Actually, the updated timeline is here: https://groups.google.com/forum/#!msg/php-fig/FU_IJjrIJSs/AtCevnZsAQAJ

> 17th January: Latest date for any PSRs to enter review under the old system

> 31st January: Latest date for any PSRs to start approval votes under the old system

Honestly, we are not rushing anything. The PSR was been discussed for years, first under container-interop and now in PSR-11. It went through 2 review periods and we are just discussing a minor issue (IMHO)

++
David.