[GitHub] [accumulo] milleruntime commented on a change in pull request #1796: Make TabletLocationState human readable

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

milleruntime commented on a change in pull request #1796:
URL: https://github.com/apache/accumulo/pull/1796#discussion_r527236251



##########
File path: core/src/main/java/org/apache/accumulo/core/metadata/TabletLocationState.java
##########
@@ -89,42 +89,39 @@ public String toString() {
 
   public TServerInstance getLocation() {
     TServerInstance result = null;
-    if (current != null) {
+    if (hasCurrent()) {
       result = current;
-    } else if (future != null) {
+    } else if (hasFuture()) {
       result = future;
     } else {
       result = last;
     }
     return result;
   }
 
-  private static final int _HAS_CURRENT = 1 << 0;
-  private static final int _HAS_FUTURE = 1 << 1;
-  private static final int _HAS_SUSPEND = 1 << 2;
+  public boolean hasCurrent() {
+    return current != null;
+  }
+
+  public boolean hasFuture() {
+    return future != null;
+  }
+
+  public boolean hasSuspend() {
+    return suspend != null;
+  }
 
   public TabletState getState(Set<TServerInstance> liveServers) {
-    switch ((current == null ? 0 : _HAS_CURRENT) | (future == null ? 0 : _HAS_FUTURE)
-        | (suspend == null ? 0 : _HAS_SUSPEND)) {
-      case 0:
-        return TabletState.UNASSIGNED;
-
-      case _HAS_SUSPEND:
-        return TabletState.SUSPENDED;
-
-      case _HAS_FUTURE:
-      case (_HAS_FUTURE | _HAS_SUSPEND):
-        return liveServers.contains(future) ? TabletState.ASSIGNED
-            : TabletState.ASSIGNED_TO_DEAD_SERVER;
-
-      case _HAS_CURRENT:
-      case (_HAS_CURRENT | _HAS_SUSPEND):
-        return liveServers.contains(current) ? TabletState.HOSTED
-            : TabletState.ASSIGNED_TO_DEAD_SERVER;
-
-      default:
-        // Both current and future are set, which is prevented by constructor.
-        throw new IllegalStateException();
-    }
+    TabletState state = TabletState.UNASSIGNED;
+    if (hasFuture())
+      state =
+          liveServers.contains(future) ? TabletState.ASSIGNED : TabletState.ASSIGNED_TO_DEAD_SERVER;
+    else if (hasCurrent())
+      state =
+          liveServers.contains(current) ? TabletState.HOSTED : TabletState.ASSIGNED_TO_DEAD_SERVER;
+    else if (hasSuspend())
+      state = TabletState.SUSPENDED;
+
+    return state;

Review comment:
       That's funny you prefer the multiple returns because I originally had this but changed it to the local variable thinking it was more readable.  Hopefully I can just hit undo a few time in my IDE...




----------------------------------------------------------------
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