Crell / Tukio

A complete and robust implementation of the PSR-14 EventDispatcher specification.
Other
105 stars 6 forks source link

Split provider interface #2

Closed Crell closed 5 years ago

Crell commented 6 years ago

An example of what the implications would be of splitting the provider into two separate interfaces.

A number of the classes involved here are tests, so the change is not, necessarily, quite as large as it looks. However, it is still extremely large.

The purported benefits of splitting the provider interface are: 1) We don't have to come up with a new name for "Event" if we rename Task to Event. 2) Users can have more targeted providers for each pipeline, since no listener will ever apply to both object types, which can be more performant. 3) It's confusing to have a single provider for 2 different things.

For points 2 and 3, that is trivially resolved by simply instantiating the provider twice. There is absolutely nothing in the spec (when the provider is not split) that forces an implementer to use the same provider object for both cases. That solves the performance question completely without any changes.

Nor is there anything that requires an implementer to have only one implementation; with a single interface it is still absolutely spec-legal to have two separate provider implementations, with different internal capabilities. Or they can use the same provider implementation. That's entirely up to the implementer to work out as they find most useful. By forcing it to be 2 separate interfaces, however, it becomes impossible to combine any part of the supporting code in a meaningful way.

(This can also be further clarified in the spec or meta-document.)

In fact, it would be super simple to implement a BasicProvider object that is essentially the RegisterableNotificationListenerProvider shown here and offer it as an alternative. Users/implementers could use either one in either case. (Say, if they want Tasks but don't care about order.) But by using the same interface, 2/3 of the code duplication shown here goes away.

In short, I see many, many downsides to forcing two separate interfaces and only marginal benefit: That we can rename Task to Event without having to come up with a new name for the generic case. I find that extremely uncompelling. At this point in the process, if we want to s/Task/Event/ I would say the onus is on those arguing to do so to come up with an alternative name. :smile: The net effect of avoiding it is substantially negative.