[GitHub] [accumulo] ctubbsii commented on a change in pull request #1651: #1473 Move TabletsMetadata builder to Ample

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

ctubbsii commented on a change in pull request #1651:
URL: https://github.com/apache/accumulo/pull/1651#discussion_r492907530



##########
File path: core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletsMetadata.java
##########
@@ -79,6 +79,7 @@
     private boolean checkConsistency = false;
     private boolean saveKeyValues;
     private TableId tableId;
+    public AccumuloClient _client;

Review comment:
       public non-final variables should be avoided.

##########
File path: core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletsMetadata.java
##########
@@ -93,6 +94,20 @@ public TabletsMetadata build(AccumuloClient client) {
       }
     }
 
+    // No-parameter build method that allows the use of AmpleImpl's Accumulo client.
+    // Ample methods can assign AmpleImple's Accumulo client to _client of this Builder object.
+    public TabletsMetadata build() {
+      Preconditions.checkState(level == null ^ table == null);

Review comment:
       This might be slightly more readable as:
   ```suggestion
         Preconditions.checkState((level == null) != (table == null));
   ```

##########
File path: core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletsMetadata.java
##########
@@ -371,6 +388,10 @@ private TabletsMetadata(Scanner scanner, Iterable<TabletMetadata> tmi) {
     this.tablets = tmi;
   }
 
+  public TabletsMetadata() {
+
+  }
+

Review comment:
       This doesn't appear to be used. Not sure it's needed.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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