zendframework / zend-stratigility

Middleware for PHP built on top of PSR-7 and PSR-15
BSD 3-Clause "New" or "Revised" License
235 stars 57 forks source link

Reuse code in MiddlewarePipe from Next #178

Closed snapshotpl closed 5 years ago

snapshotpl commented 6 years ago

I had refactor step by step (commit by commit) so you can reviewing this pr by commits.

By first step I remove duplication of Next and MiddlewarePipe - Next can be deprecated. Then I replace SplQueue to array - IMO they are fasters however I didn't any benchmarks. I last change I move Next logic into Handle method - for improved readability.

snapshotpl commented 6 years ago

@weierophinney Following your suggestions I rollback and improve my main change: reuse Next in MiddlewarePipe.

Build fail because __clone is not covered now (Next care about clone queue). We can delete this implementation now, however it can be BC.

There is another option to throw EmptyPipelineException: by anonymus class, but current implementation is more readable for me.

snapshotpl commented 6 years ago

@weierophinney I'm waiting for your response here :)

weierophinney commented 6 years ago

@snapshotpl — regarding this exchange (github is not giving me a reply button on that comment, unfortunately!):

With this change, the exception may be raised even if there are middleware in the pipeline, if the last middleware calls its $handler.

What was the previous behavior?

I already detailed that in the previous comment:

Previously, we detected if the pipeline were empty before attempting to process() the request, and raised an exception in that situation.