[GitHub] mikewalch commented on issue #747: Shorten method names in AccumuloClient builder

[GitBox <gi...@apache.org>]

mikewalch commented on issue #747: Shorten method names in AccumuloClient builder
URL: https://github.com/apache/accumulo/issues/747#issuecomment-436049107
 
 
   Below is my second proposal incorporating a lot of @ctubbsii suggestions with some differences:
   
   * Kept instance name & zookeeper together in the same method as they are always both needed.
   * I like the prepositional phrases early but I don't think they are required later in the builder.
   * I renamed `forInstance` to `to` but maybe `toCluster` or even the original `forInstance` is better.
   
   ```java
   // current
   Accumulo.newClient().forInstance("myinstance", "zookeeper1,zookeeper2")
         .usingPassword("myuser", "mypassword").build();
   // proposed
   Accumulo.newClient().to("myinstance", "zookeeper1,zookeeper2")
         .as("myuser").password("mypassword").build();
   
   // current
   Accumulo.newClient().usingClientInfo(clientInfo)
        .usingToken("user, new PasswordToken("pass")).build();
   // proposed
   Accumulo.newClient().from(clientInfo).as("user").token(new PasswordToken("pass")).build();
   ```
   
   | Current Method Name | Proposed Method Name |
   |--------------------------|-------------------------------|
   | forInstance(String name, String zk) | to(CharSequence instanceName, CharSequence zookeepers) |
   | usingProperties(String propsFilePath) | from(Path propsFile) |
   | usingProperties(Properties properties) | from(Properties props) |
   | usingClientInfo(ClientInfo info) | from(ClientInfo info) |
   | doesn't exist | as(CharSequence principal) |
   | usingPassword(String username, CharSequence password) | as(user).password(CharSequence password) |
   | usingKerberos(String principal, String keytabFile) | as(user).kerberos(Path keytab) |
   | usingToken(String principal, AuthenticationToken token) | as(user).token(AuthenticationToken token) |
   | withTruststore | truststore |
   | withKeystore | keystore |
   | withJsse | jsse |
   | withPrimary | primary |
   | withQop | qop |
   | withSsl | ssl |
   | withSasl | sasl |
   | withBatchWriterConfig | batchWriterConfig |
   | withBatchScannerQueryThreads | batchScannerQueryThreads |
   | withScannerBatchSize | scannerBatchSize |
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services