Inconsistent names and descriptions

[mindplay-dk]

MiddlewareInterface has this signature:

public function process(RequestInterface $request, DelegateInterface $next);

But ServerMiddlewareInterface has this signature:

public function process(ServerRequestInterface $request, DelegateInterface $frame);

The delegate is $next in one place and $frame in another.

I don't recall precisely what we decided, but I think there was general agreement not to refer to the delegate as a "frame"? Inline documentation for ServerMiddlewareInterface also presently says:

Takes the incoming request and optionally modifies it before delegating
to the next frame to get a response.

I would suggest the following:

Takes the incoming Request and returns a Response, optionally delegating
to the next middleware component to create the Response.

Also, the DelegateInterface method is named next(), so when middleware code delegates to the next middleware, it comes out return $next->next($request), which is not very semantic - something like return $next->delegate($request) or return $delegate->process($request) would be more meaningful.

(I don't think using the name process() in the middleware-delegate would be an issue? I can't imagine a case where a middleware delegate implementation is also a middleware component, can you?)

Finally, the first line of the description of MiddlewareInterface reads:

Process a client request and return a response.

But this interface is for middleware that works on both for client and server-requests, right? So:

Process a client or server Request and return a Response.

@http-interop/http-middleware-contributors I can PR these changes if there is no disagreement? Let me know.

[shadowhand]

The delegate is $next in one place and $frame in another.

I am in favor of:

DelegateInterface $delegate

Finally, the first line of the description of MiddlewareInterface reads: [ ... ]

I don't think that Response and Request need to be capitalized in descriptions, otherwise I agree with your suggestions.

[mindplay-dk]

I have a habit of capitalizing domain terms, but maybe that's just me.

[weierophinney]

:+1: to DelegateInterface $delegate.

[mindplay-dk]

I removed the capitalization in this PR, and didn't change the method name, so it'll now come out as e.g. return $delegate->next($request) which seems reasonable.

[mindplay-dk]

I'm unsure which signature provides the most meaningful semantics - please see an example of two different terminologies applied here:

https://gist.github.com/mindplay-dk/0b24ad9ed73383e5b723847c545a26bc

The last line of each option is what delegation inside middleware would look like.

@http-interop/http-middleware-contributors please reply with A or B here, and I will update the PR accordingly?

Thanks.

[weierophinney]

I prefer option B; the delegate is handing off processing to the next layer.

On Sep 17, 2016 9:21 AM, "Rasmus Schultz" notifications@github.com wrote:

I'm unsure which signature provides the most meaningful semantics - please see an example of two different terminologies applied here:

https://gist.github.com/mindplay-dk/0b24ad9ed73383e5b723847c545a26bc

The last line of each option is what delegation inside middleware would look like.

@http-interop/http-middleware-contributors https://github.com/orgs/http-interop/teams/http-middleware-contributors please reply with A or B here, and I will update the PR accordingly?

Thanks.

— You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub https://github.com/http-interop/http-middleware/issues/15#issuecomment-247774536, or mute the thread https://github.com/notifications/unsubscribe-auth/AABlVwXZoMPoE23JPMyrs8Qdce5x8fLLks5qq_dygaJpZM4Ju5ie .

[mindplay-dk]

@weierophinney are you sure about the method-name though? it does not imply any action. I feel like there should be a verb in there somewhere?

processNext() is a little more verbose, but more consistent with process() in the middleware interface maybe?

[oscarotero]

To me, a method called next() has not sense here, because the DelegateInteface is not a queue or a stack of elements that you can move (there's no a prev(), or if you call next() twice the same compontent is executed two times instead move two positions). I interpret the DelegateInterface as the "next" itself, so the only thing you can do is execute it. To me $next->process() or $delegate->process() is more appropiate. And even, if the DeletageInterface is callable (as the middleman implementation $next() is more meaningful, so my preferred signature is $next->process($request) / $next($request)

[mindplay-dk]

@oscarotero you make some very good points.

$next->process() would be consistent with MiddlewareInterface::process(), which is really what both of those methods do - one just happens to be a delegate to middleware which then processes, but the net result of calling either method are identical.

The only pitfall here is, are we overlooking a case where the same class would need to implement DelegateInterface and MiddlewareInterface for some reason?

Probably not - I don't think it would make much sense.

But that, in my opinion, would be the only reason not to use the same terminology.

[shadowhand]

Renaming DelegateInterface::next() to process() would be technically incorrect, as the delegate is not responsible for processing, but only for delegating to the next available middleware. The semantics are pretty obvious, in my opinion:

// Delegate response creation to the next available middleware
return $delegate->next($request);

[mindplay-dk]

Technically incorrect?

In my experience it's completely normal to name a method according to what the method does - whether it delegates to some other method for the actual work, in my opinion, is completely beside the point. Most implementations delegate some part or all of their work to some other function or method.

In my opinion, a method-name should describe the action you're performing when you call the method.

Client code is asking the delegate to process the request - because it's a delegate, it delegates that responsibility to some other party, but that's an implementation detail; from the consumer's point of view, you're asking the delegate to process the request.

[shadowhand]

I guess that's fair. @oscarotero can you make a PR please?

[oscarotero]

@shadowhand PR created #24 I'm not sure if you want to rename also the variable $delegate to $next or not (As I can see, it was renamed previously from $next to $delegate)

[shadowhand]

@oscarotero thanks, merged!

[mindplay-dk]

Happy with this now. Close?

[mindplay-dk]

@shadowhand and the next release will be 0.2.0 since the method-name change was a breaking change, yeah?

[shadowhand]

Released 0.2.0.

[mindplay-dk]

FYI, middleman 2.0 now supports/requires ^0.2