[jira] [Commented] (CURATOR-148) Order of modifiers on framework commands matters

["Mike Drob (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CURATOR-148?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14505922#comment-14505922 ] 

Mike Drob commented on CURATOR-148:
-----------------------------------

[~randgalt]

On the ML you said you'd like this to be part of a larger usability sweep. Can you elaborate?

> Order of modifiers on framework commands matters
> ------------------------------------------------
>
>                 Key: CURATOR-148
>                 URL: https://issues.apache.org/jira/browse/CURATOR-148
>             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
(v6.3.4#6332)