zendframework / zend-expressive

PSR-15 middleware in minutes!
BSD 3-Clause "New" or "Revised" License
711 stars 197 forks source link

Move more code to zend-expressive-router? #205

Closed mtymek closed 8 years ago

mtymek commented 8 years ago

Since almost the beginning, I have a feeling that Application class does too much with routing, which breaks SRP. I thought that it is too late for architectural changes, but seeing that this package is being split today, I suggest moving more stuff away from this class, to zend-expressive-router package:

weierophinney commented 8 years ago

The primary reasons we didn't do this has disappeared: container awareness. Originally, the routes would return the middleware, which meant the router needed awareness of the container; we were injecting it in the AppFactory.

However, there are other reasons we've kept the separation:

While I agree that some of these decisions break SRP, the reasons for doing so are largely to ensure that differences in router implementations do not lead to substantial differences in behavior. Doing so reduces maintenance costs significantly by removing edge cases.

mtymek commented 8 years ago

Thanks for your explanation - it makes sense for me now. Personally I would probably design it in different way, but I will leave this thoughts for v2.0 discussion, somewhere in the future :-)

weierophinney commented 8 years ago

@mtymek I'd also recommend reading this:

-zendframework/zend-expressive#216#issuecomment-162923176

as I think that gives a more thorough breakdown.

Essentially, Application is a facade, and facades are a specific case where SRP can and should be violated.