curator-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mike Drob (JIRA)" <>
Subject [jira] [Commented] (CURATOR-148) Order of modifiers on framework commands matters
Date Fri, 12 Sep 2014 02:56:33 GMT


Mike Drob commented on CURATOR-148:

I understand that not all combinations make sense. However, I don't understand how {{withMode().withACL()}}
would be any different from {{withACL().withMode()}} from the caller's perspective.

I'll do a more thorough sweep of the API tomorrow or possibly next week. Once we establish
which ones are intentionally left out, I'll be happy to submit a PR to add in the rest if
that's the cleanest solution.

> Order of modifiers on framework commands matters
> ------------------------------------------------
>                 Key: CURATOR-148
>                 URL:
>             Project: Apache Curator
>          Issue Type: Improvement
>          Components: Client
>            Reporter: Mike Drob
> I only tested this with the {{CreateBuilder}}, but visual inspection makes me think that
it is present on other operations as well.
> If I want to create a ZK node, using a bunch of the features that curator provides, I
have to do it in a specific order. (Arguments removed for clarity)
> {code}
> client.create()
>     .compressed()
>     .withMode()
>     .withACL()
>     .inBackground()
>     .forPath();
> {code}
> If I unknowingly call {{inBackground()}} first, then the only methods available to me
are the {{forPath()}} variants. Similarly, if I call {{create().withMode()}}, then there is
longer a way for me to call {{compressed()}} on that object.
> Even more concerning is that it is impossible to call {{compressed()}} and {{withProtection()}}
on the same chain, regardless of order.
> Since each of the fluent-style methods already returns {{this}}, it might make sense
to modify each method as implemented on {{CreateBuilder}} to have a declared return type of
{{CreateBuilder}}, taking advantage of covariant return types.
> Another option, with similar results, would be to remove many of the intermediate interfaces
from {{CreateBuilder}} like {{ACLCreateModeBackgroundPathAndBytesable<String>}} and
declare it to be: 
> {code}
> interface CreateBuilder extends Pathable<String>,
>     Backgroundable<CreateBuilder>,
>     Compressible<CreateBuilder>,
>     ACLable<CreateBuilder>,
>     CreateModable<CreateBuilder>,
>     ....
> {code}
> This option is very verbose, however.
> A disadvantage of both of these options is that it allows users to call methods multiple
times. In some cases, like {{inBackground()}}, it won't matter. In other cases, like {{withACLs()}}
we'd have to make a decision on taking an intersection or "last call wins" approach. That
might even differ per call, so we'd have to have careful documentation on each.
> Another option is to simply document the behavior (probably on the {{create()}} method)
and hope that users will see it. Maybe the best solution is to document in a minor release
line, and then make breaking changes in a major version?

This message was sent by Atlassian JIRA

View raw message