ctubbsii commented on issue #747: Shorten method names in AccumuloClient builder URL: https://github.com/apache/accumulo/issues/747#issuecomment-436078424 @mikewalch I like the `to(name)`, because we're connecting *to* an Accumulo instance with that name, but the second parameter could be misleading with that name (we're not constructing a client *to* those hosts; they just serve as an endpoint), and I'm not a fan of two string types like that... it's very easy to accidentally swap them (common problem and why we made the shell have explicit separate args, `zi` and `zh` to replace `zk`). Maybe we should do: ```java to(CharSequence name, CharSequence[] zookeepers); ``` With the array type, and using Java 8 syntax, we can call it like: ```java Accumulo.newClient().to(name, {"localhost:2181"}); // OR Accumulo.newClient().to(name, {zk1, zk2}); ``` This is a bit more explicit about the ZKs, and less ambiguous in the params. Also, the more I think about it, the more strongly I think that `from(String propsFilePath)` should be `from(Path propsFilePath)`, especially for consistency with the other place (keytab) where you use `Path` type. I don't think this will bloat user code too much... it's (at the most) once per client... and if a variable is used, it doesn't even matter: ```java Path propsFile = Paths.get(args[0]); Accumulo.newClient().from(propsFile); ``` Also, the "jsse" method is confusing. That should just be a boolean that says "use JSSE system props". Maybe "useJsse" makes more sense than just "jsse". I think some of the others have similarly bad semantics when removing "with". They just don't read as well, or make as much intuitive sense.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
