cover question marks more robustly

apache · Dec 16, 2024 · 474dc72 · 474dc72
1 parent ec0e2cc
commit 474dc72
Showing 1 changed file with 35 additions and 18 deletions.
diff --git a/src/java/org/apache/cassandra/tools/nodetool/Status.java b/src/java/org/apache/cassandra/tools/nodetool/Status.java
@@ -28,6 +28,7 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.SortedMap;
 import java.util.UUID;
 import java.util.function.BiFunction;
@@ -407,8 +408,15 @@ int compareByHost(Map.Entry<String, List<Object>> row1, Map.Entry<String, List<O
 
         int compareByOwns(Map.Entry<String, List<Object>> row1, Map.Entry<String, List<Object>> row2)
         {
-            double value1 = Double.parseDouble(((String) row1.getValue().get(tokenPerNode ? 4 : 5)).replace("%", ""));
-            double value2 = Double.parseDouble(((String) row2.getValue().get(tokenPerNode ? 4 : 5)).replace("%", ""));
+            String str1 = (String) row1.getValue().get(tokenPerNode ? 4 : 5);
+            String str2 = (String) row2.getValue().get(tokenPerNode ? 4 : 5);
+
+            Optional<Integer> maybeReturn = maybeCompareQuestionMarks(str1, str2);
+            if (maybeReturn.isPresent())
+                return maybeReturn.get();
+
+            double value1 = Double.parseDouble(str1.replace("%", ""));
+            double value2 = Double.parseDouble(str2.replace("%", ""));
 
             return evaluateComparision(Double.compare(value1, value2));
         }
@@ -464,24 +472,13 @@ int compareByLoad(Map.Entry<String, List<Object>> row1, Map.Entry<String, List<O
         {
             String str1 = (String) row1.getValue().get(3);
             String str2 = (String) row2.getValue().get(3);
-            // Check if str1 or str2 contains a '?' and set a value for it.
-            boolean containsQuestionMark1 = str1.contains("?");
-            boolean containsQuestionMark2 = str2.contains("?");
-            // If both contain '?', return 0 (they are considered equal).
-            if (containsQuestionMark1 && containsQuestionMark2)
-                return 0;
 
-            // If str1 contains '?', ensure it's last (or first depending on descending).
-            if (containsQuestionMark1)
-                return descending(sortOrder) ? 1 : -1;
+            Optional<Integer> maybeReturn = maybeCompareQuestionMarks(str1, str2);
+            if (maybeReturn.isPresent())
+                return maybeReturn.get();
 
-            // If str2 contains '?', ensure it's last (or first depending on descending).
-            if (containsQuestionMark2)
-                return descending(sortOrder) ? -1 : 1;
-
-            // If neither contain '?', parse the file sizes and compare.
-            long value1 = FileUtils.parseFileSize((String) row1.getValue().get(3));
-            long value2 = FileUtils.parseFileSize((String) row2.getValue().get(3));
+            long value1 = FileUtils.parseFileSize(str1);
+            long value2 = FileUtils.parseFileSize(str2);
 
             return evaluateComparision(Long.compare(value1, value2));
         }
@@ -504,5 +501,25 @@ private int evaluateComparision(int comparisionResult)
 
             return 0;
         }
+
+        private Optional<Integer> maybeCompareQuestionMarks(String str1, String str2)
+        {
+            // Check if str1 or str2 contains a '?' and set a value for it.
+            boolean containsQuestionMark1 = str1.contains("?");
+            boolean containsQuestionMark2 = str2.contains("?");
+            // If both contain '?', return 0 (they are considered equal).
+            if (containsQuestionMark1 && containsQuestionMark2)
+                return Optional.of(0);
+
+            // If str1 contains '?', ensure it's last (or first depending on descending).
+            if (containsQuestionMark1)
+                return Optional.of(descending(sortOrder) ? 1 : -1);
+
+            // If str2 contains '?', ensure it's last (or first depending on descending).
+            if (containsQuestionMark2)
+                return Optional.of(descending(sortOrder) ? -1 : 1);
+
+            return Optional.empty();
+        }
     }
 }