jrwest commented on code in PR #3733:
URL: https://github.com/apache/cassandra/pull/3733#discussion_r1886166443


##########
src/java/org/apache/cassandra/tools/nodetool/Status.java:
##########
@@ -24,38 +25,232 @@
 import java.io.PrintStream;
 import java.net.UnknownHostException;
 import java.text.DecimalFormat;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.SortedMap;
 
+import org.apache.cassandra.io.util.FileUtils;
 import org.apache.cassandra.locator.EndpointSnitchInfoMBean;
+import org.apache.cassandra.locator.InetAddressAndPort;
 import org.apache.cassandra.tools.NodeProbe;
 import org.apache.cassandra.tools.NodeTool;
 import org.apache.cassandra.tools.NodeTool.NodeToolCmd;
 import org.apache.cassandra.tools.nodetool.formatter.TableBuilder;
 
 import com.google.common.collect.ArrayListMultimap;
 
+import static java.util.stream.Collectors.toMap;
+
 @SuppressWarnings("UseOfSystemOutOrSystemErr")
 @Command(name = "status", description = "Print cluster information (state, 
load, IDs, ...)")
-public class Status extends NodeToolCmd
-{
+public class Status extends NodeToolCmd {
     @Arguments(usage = "[<keyspace>]", description = "The keyspace name")
     private String keyspace = null;
 
     @Option(title = "resolve_ip", name = {"-r", "--resolve-ip"}, description = 
"Show node domain names instead of IPs")
     private boolean resolveIp = false;
 
+    @Option(title = "sort",
+            name = {"-s", "--sort"},
+            description = "Sort by one of 'ip', 'load', 'owns', 'id', 'rack' 
or 'state', Defaults to 'none', Default Ordering is 'asc' for 'ip', 'id', 
'rack' and 'desc' for 'load', 'owns', 'state' ",
+            allowedValues = {"ip", "load", "owns", "id", "rack", "state", 
"none"})
+    private SortBy sortBy = SortBy.none;

Review Comment:
   Making these values enums makes the error messages when you pass an invalid 
value a bit opaque. On one hand, I would say its fine to leave it because the 
output tells you to check `nodetool help` which does tell you the valid values. 
OTOH, having it tell you the valid values in the error message would be a 
better user experience (and a removal of a rough edge nodetool often has). To 
fix this though I think we would need to take the arguments as strings and then 
parse them in the body of the command so we could catch the exception and throw 
our own error message:
   
   ```
   $ nt status --sort blah
   nodetool: sort: can not convert "blah" to a SortBy
   See 'nodetool help' or 'nodetool help <command>'.
   
   $ nt status --sort ip --order blah
   nodetool: sort_order: can not convert "blah" to a SortOrder
   See 'nodetool help' or 'nodetool help <command>'.
   ```



##########
src/java/org/apache/cassandra/tools/nodetool/Status.java:
##########
@@ -24,38 +25,232 @@
 import java.io.PrintStream;
 import java.net.UnknownHostException;
 import java.text.DecimalFormat;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.SortedMap;
 
+import org.apache.cassandra.io.util.FileUtils;
 import org.apache.cassandra.locator.EndpointSnitchInfoMBean;
+import org.apache.cassandra.locator.InetAddressAndPort;
 import org.apache.cassandra.tools.NodeProbe;
 import org.apache.cassandra.tools.NodeTool;
 import org.apache.cassandra.tools.NodeTool.NodeToolCmd;
 import org.apache.cassandra.tools.nodetool.formatter.TableBuilder;
 
 import com.google.common.collect.ArrayListMultimap;
 
+import static java.util.stream.Collectors.toMap;
+
 @SuppressWarnings("UseOfSystemOutOrSystemErr")
 @Command(name = "status", description = "Print cluster information (state, 
load, IDs, ...)")
-public class Status extends NodeToolCmd
-{
+public class Status extends NodeToolCmd {
     @Arguments(usage = "[<keyspace>]", description = "The keyspace name")
     private String keyspace = null;
 
     @Option(title = "resolve_ip", name = {"-r", "--resolve-ip"}, description = 
"Show node domain names instead of IPs")
     private boolean resolveIp = false;
 
+    @Option(title = "sort",
+            name = {"-s", "--sort"},
+            description = "Sort by one of 'ip', 'load', 'owns', 'id', 'rack' 
or 'state', Defaults to 'none', Default Ordering is 'asc' for 'ip', 'id', 
'rack' and 'desc' for 'load', 'owns', 'state' ",
+            allowedValues = {"ip", "load", "owns", "id", "rack", "state", 
"none"})
+    private SortBy sortBy = SortBy.none;
+
+    @Option(title = "sort_order",
+            name = {"-o", "--order"},
+            description = "Sort order: 'asc' for ascending, 'desc' for 
descending",
+            allowedValues = {"asc", "desc"})
+    private SortOrder sortOrder = null;
+
     private boolean isTokenPerNode = true;
     private Collection<String> joiningNodes, leavingNodes, movingNodes, 
liveNodes, unreachableNodes;
     private Map<String, String> loadMap, hostIDMap;
     private EndpointSnitchInfoMBean epSnitchInfo;
 
+    public enum SortOrder {
+        asc,
+        desc
+    }
+
+    public enum SortBy
+    {
+        none
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data;
+                    }
+                },
+        ip(false)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data.entrySet()
+                                .stream()
+                                .sorted((e1, e2) -> {
+                                    InetAddressAndPort addr1 = 
(InetAddressAndPort) e1.getValue().get(0);
+                                    InetAddressAndPort addr2 = 
(InetAddressAndPort) e2.getValue().get(0);
+
+                                    return 
evaluateComparision(addr1.compareTo(addr2));
+                                })
+                                .collect(toMap(Map.Entry::getKey, 
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+                    }
+                },
+        load(true)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data.entrySet()
+                                .stream()
+                                .sorted((e1, e2) -> {
+                                    String str1 = (String) 
e1.getValue().get(3);
+                                    String str2 = (String) 
e2.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 (containsQuestionMark1 && 
containsQuestionMark2) {
+                                        // If both contain '?', return 0 (they 
are considered equal).
+                                        return 0;
+                                    }
+
+                                    boolean descending = descending(sortOrder);
+
+                                    // If str1 contains '?', ensure it's last 
(or first depending on descending).
+                                    if (containsQuestionMark1)
+                                        return descending ? 1 : -1;
+
+                                    // If str2 contains '?', ensure it's last 
(or first depending on descending).
+                                    if (containsQuestionMark2)
+                                        return descending ? -1 : 1;
+
+                                    // If neither contain '?', parse the file 
sizes and compare.
+                                    long value1 = 
FileUtils.parseFileSize((String) e1.getValue().get(3));
+                                    long value2 = 
FileUtils.parseFileSize((String) e2.getValue().get(3));
+
+                                    return 
evaluateComparision(Long.compare(value1, value2));
+                                })
+                                .collect(toMap(Map.Entry::getKey, 
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+                    }
+                },
+        owns(true)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data.entrySet()
+                                .stream()
+                                .sorted((e1, e2) -> {
+                                    double value1 = 
Double.parseDouble(((String) e1.getValue().get(tokenPerNode ? 4 : 
5)).replace("%", ""));
+                                    double value2 = 
Double.parseDouble(((String) e2.getValue().get(tokenPerNode ? 4 : 
5)).replace("%", ""));
+
+                                    return 
evaluateComparision(Double.compare(value1, value2));
+                                })
+                                .collect(toMap(Map.Entry::getKey, 
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+                    }
+                },
+        id(false)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return compareStrings(data, tokenPerNode ? 5 : 6);
+                    }
+                },
+        rack(false)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return compareStrings(data, 7);
+                    }
+                },
+        state(true)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return compareStrings(data, 1);
+                    }
+                };
+
+        private final boolean descendingByDefault;
+        boolean tokenPerNode;
+        SortOrder sortOrder;
+
+        SortBy()
+        {
+            this(false);
+        }
+
+        SortBy(boolean descendingByDefault)
+        {
+            this.descendingByDefault = descendingByDefault;
+        }
+
+        public abstract Map<String, List<Object>> sort(Map<String, 
List<Object>> data);
+
+        boolean descending(SortOrder sortOrder)

Review Comment:
   should be marked protected



##########
src/java/org/apache/cassandra/tools/nodetool/Status.java:
##########
@@ -24,38 +25,232 @@
 import java.io.PrintStream;
 import java.net.UnknownHostException;
 import java.text.DecimalFormat;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.SortedMap;
 
+import org.apache.cassandra.io.util.FileUtils;
 import org.apache.cassandra.locator.EndpointSnitchInfoMBean;
+import org.apache.cassandra.locator.InetAddressAndPort;
 import org.apache.cassandra.tools.NodeProbe;
 import org.apache.cassandra.tools.NodeTool;
 import org.apache.cassandra.tools.NodeTool.NodeToolCmd;
 import org.apache.cassandra.tools.nodetool.formatter.TableBuilder;
 
 import com.google.common.collect.ArrayListMultimap;
 
+import static java.util.stream.Collectors.toMap;
+
 @SuppressWarnings("UseOfSystemOutOrSystemErr")
 @Command(name = "status", description = "Print cluster information (state, 
load, IDs, ...)")
-public class Status extends NodeToolCmd
-{
+public class Status extends NodeToolCmd {
     @Arguments(usage = "[<keyspace>]", description = "The keyspace name")
     private String keyspace = null;
 
     @Option(title = "resolve_ip", name = {"-r", "--resolve-ip"}, description = 
"Show node domain names instead of IPs")
     private boolean resolveIp = false;
 
+    @Option(title = "sort",
+            name = {"-s", "--sort"},
+            description = "Sort by one of 'ip', 'load', 'owns', 'id', 'rack' 
or 'state', Defaults to 'none', Default Ordering is 'asc' for 'ip', 'id', 
'rack' and 'desc' for 'load', 'owns', 'state' ",
+            allowedValues = {"ip", "load", "owns", "id", "rack", "state", 
"none"})
+    private SortBy sortBy = SortBy.none;
+
+    @Option(title = "sort_order",
+            name = {"-o", "--order"},
+            description = "Sort order: 'asc' for ascending, 'desc' for 
descending",
+            allowedValues = {"asc", "desc"})
+    private SortOrder sortOrder = null;
+
     private boolean isTokenPerNode = true;
     private Collection<String> joiningNodes, leavingNodes, movingNodes, 
liveNodes, unreachableNodes;
     private Map<String, String> loadMap, hostIDMap;
     private EndpointSnitchInfoMBean epSnitchInfo;
 
+    public enum SortOrder {
+        asc,
+        desc
+    }
+
+    public enum SortBy

Review Comment:
   If I am reading these right, each implementation duplicates a lot of the 
same code in their `sort` implementations. The only difference is the 
comparator. We could instead define the comparator and have one sort 
implementation which would reduce a lot of duplication and LOC.



##########
src/java/org/apache/cassandra/tools/nodetool/Status.java:
##########
@@ -24,38 +25,232 @@
 import java.io.PrintStream;
 import java.net.UnknownHostException;
 import java.text.DecimalFormat;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.SortedMap;
 
+import org.apache.cassandra.io.util.FileUtils;
 import org.apache.cassandra.locator.EndpointSnitchInfoMBean;
+import org.apache.cassandra.locator.InetAddressAndPort;
 import org.apache.cassandra.tools.NodeProbe;
 import org.apache.cassandra.tools.NodeTool;
 import org.apache.cassandra.tools.NodeTool.NodeToolCmd;
 import org.apache.cassandra.tools.nodetool.formatter.TableBuilder;
 
 import com.google.common.collect.ArrayListMultimap;
 
+import static java.util.stream.Collectors.toMap;
+
 @SuppressWarnings("UseOfSystemOutOrSystemErr")
 @Command(name = "status", description = "Print cluster information (state, 
load, IDs, ...)")
-public class Status extends NodeToolCmd
-{
+public class Status extends NodeToolCmd {
     @Arguments(usage = "[<keyspace>]", description = "The keyspace name")
     private String keyspace = null;
 
     @Option(title = "resolve_ip", name = {"-r", "--resolve-ip"}, description = 
"Show node domain names instead of IPs")
     private boolean resolveIp = false;
 
+    @Option(title = "sort",
+            name = {"-s", "--sort"},
+            description = "Sort by one of 'ip', 'load', 'owns', 'id', 'rack' 
or 'state', Defaults to 'none', Default Ordering is 'asc' for 'ip', 'id', 
'rack' and 'desc' for 'load', 'owns', 'state' ",
+            allowedValues = {"ip", "load", "owns", "id", "rack", "state", 
"none"})
+    private SortBy sortBy = SortBy.none;
+
+    @Option(title = "sort_order",
+            name = {"-o", "--order"},
+            description = "Sort order: 'asc' for ascending, 'desc' for 
descending",
+            allowedValues = {"asc", "desc"})
+    private SortOrder sortOrder = null;
+
     private boolean isTokenPerNode = true;
     private Collection<String> joiningNodes, leavingNodes, movingNodes, 
liveNodes, unreachableNodes;
     private Map<String, String> loadMap, hostIDMap;
     private EndpointSnitchInfoMBean epSnitchInfo;
 
+    public enum SortOrder {
+        asc,
+        desc
+    }
+
+    public enum SortBy
+    {
+        none
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data;
+                    }
+                },
+        ip(false)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data.entrySet()
+                                .stream()
+                                .sorted((e1, e2) -> {
+                                    InetAddressAndPort addr1 = 
(InetAddressAndPort) e1.getValue().get(0);
+                                    InetAddressAndPort addr2 = 
(InetAddressAndPort) e2.getValue().get(0);
+
+                                    return 
evaluateComparision(addr1.compareTo(addr2));
+                                })
+                                .collect(toMap(Map.Entry::getKey, 
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+                    }
+                },
+        load(true)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data.entrySet()
+                                .stream()
+                                .sorted((e1, e2) -> {
+                                    String str1 = (String) 
e1.getValue().get(3);
+                                    String str2 = (String) 
e2.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 (containsQuestionMark1 && 
containsQuestionMark2) {
+                                        // If both contain '?', return 0 (they 
are considered equal).
+                                        return 0;
+                                    }
+
+                                    boolean descending = descending(sortOrder);
+
+                                    // If str1 contains '?', ensure it's last 
(or first depending on descending).
+                                    if (containsQuestionMark1)
+                                        return descending ? 1 : -1;
+
+                                    // If str2 contains '?', ensure it's last 
(or first depending on descending).
+                                    if (containsQuestionMark2)
+                                        return descending ? -1 : 1;
+
+                                    // If neither contain '?', parse the file 
sizes and compare.
+                                    long value1 = 
FileUtils.parseFileSize((String) e1.getValue().get(3));
+                                    long value2 = 
FileUtils.parseFileSize((String) e2.getValue().get(3));
+
+                                    return 
evaluateComparision(Long.compare(value1, value2));
+                                })
+                                .collect(toMap(Map.Entry::getKey, 
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+                    }
+                },
+        owns(true)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data.entrySet()
+                                .stream()
+                                .sorted((e1, e2) -> {
+                                    double value1 = 
Double.parseDouble(((String) e1.getValue().get(tokenPerNode ? 4 : 
5)).replace("%", ""));
+                                    double value2 = 
Double.parseDouble(((String) e2.getValue().get(tokenPerNode ? 4 : 
5)).replace("%", ""));
+
+                                    return 
evaluateComparision(Double.compare(value1, value2));
+                                })
+                                .collect(toMap(Map.Entry::getKey, 
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+                    }
+                },
+        id(false)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return compareStrings(data, tokenPerNode ? 5 : 6);
+                    }
+                },
+        rack(false)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return compareStrings(data, 7);
+                    }
+                },
+        state(true)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return compareStrings(data, 1);
+                    }
+                };
+
+        private final boolean descendingByDefault;
+        boolean tokenPerNode;
+        SortOrder sortOrder;
+
+        SortBy()
+        {
+            this(false);
+        }
+
+        SortBy(boolean descendingByDefault)
+        {
+            this.descendingByDefault = descendingByDefault;
+        }
+
+        public abstract Map<String, List<Object>> sort(Map<String, 
List<Object>> data);
+
+        boolean descending(SortOrder sortOrder)
+        {
+            return sortOrder == null ? descendingByDefault : sortOrder == 
SortOrder.desc;
+        }
+
+        SortBy sortOrder(SortOrder sortOrder)
+        {
+            this.sortOrder = sortOrder;
+            return this;
+        }
+
+        SortBy tokenPerNode(boolean tokenPerNode)

Review Comment:
   should be marked private or protected



##########
src/java/org/apache/cassandra/tools/nodetool/Status.java:
##########
@@ -24,38 +25,232 @@
 import java.io.PrintStream;
 import java.net.UnknownHostException;
 import java.text.DecimalFormat;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.SortedMap;
 
+import org.apache.cassandra.io.util.FileUtils;
 import org.apache.cassandra.locator.EndpointSnitchInfoMBean;
+import org.apache.cassandra.locator.InetAddressAndPort;
 import org.apache.cassandra.tools.NodeProbe;
 import org.apache.cassandra.tools.NodeTool;
 import org.apache.cassandra.tools.NodeTool.NodeToolCmd;
 import org.apache.cassandra.tools.nodetool.formatter.TableBuilder;
 
 import com.google.common.collect.ArrayListMultimap;
 
+import static java.util.stream.Collectors.toMap;
+
 @SuppressWarnings("UseOfSystemOutOrSystemErr")
 @Command(name = "status", description = "Print cluster information (state, 
load, IDs, ...)")
-public class Status extends NodeToolCmd
-{
+public class Status extends NodeToolCmd {
     @Arguments(usage = "[<keyspace>]", description = "The keyspace name")
     private String keyspace = null;
 
     @Option(title = "resolve_ip", name = {"-r", "--resolve-ip"}, description = 
"Show node domain names instead of IPs")
     private boolean resolveIp = false;
 
+    @Option(title = "sort",
+            name = {"-s", "--sort"},
+            description = "Sort by one of 'ip', 'load', 'owns', 'id', 'rack' 
or 'state', Defaults to 'none', Default Ordering is 'asc' for 'ip', 'id', 
'rack' and 'desc' for 'load', 'owns', 'state' ",
+            allowedValues = {"ip", "load", "owns", "id", "rack", "state", 
"none"})
+    private SortBy sortBy = SortBy.none;
+
+    @Option(title = "sort_order",
+            name = {"-o", "--order"},
+            description = "Sort order: 'asc' for ascending, 'desc' for 
descending",
+            allowedValues = {"asc", "desc"})
+    private SortOrder sortOrder = null;
+
     private boolean isTokenPerNode = true;
     private Collection<String> joiningNodes, leavingNodes, movingNodes, 
liveNodes, unreachableNodes;
     private Map<String, String> loadMap, hostIDMap;
     private EndpointSnitchInfoMBean epSnitchInfo;
 
+    public enum SortOrder {
+        asc,
+        desc
+    }
+
+    public enum SortBy
+    {
+        none
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data;
+                    }
+                },
+        ip(false)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data.entrySet()
+                                .stream()
+                                .sorted((e1, e2) -> {
+                                    InetAddressAndPort addr1 = 
(InetAddressAndPort) e1.getValue().get(0);
+                                    InetAddressAndPort addr2 = 
(InetAddressAndPort) e2.getValue().get(0);
+
+                                    return 
evaluateComparision(addr1.compareTo(addr2));
+                                })
+                                .collect(toMap(Map.Entry::getKey, 
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+                    }
+                },
+        load(true)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data.entrySet()
+                                .stream()
+                                .sorted((e1, e2) -> {
+                                    String str1 = (String) 
e1.getValue().get(3);
+                                    String str2 = (String) 
e2.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 (containsQuestionMark1 && 
containsQuestionMark2) {
+                                        // If both contain '?', return 0 (they 
are considered equal).
+                                        return 0;
+                                    }
+
+                                    boolean descending = descending(sortOrder);
+
+                                    // If str1 contains '?', ensure it's last 
(or first depending on descending).
+                                    if (containsQuestionMark1)
+                                        return descending ? 1 : -1;
+
+                                    // If str2 contains '?', ensure it's last 
(or first depending on descending).
+                                    if (containsQuestionMark2)
+                                        return descending ? -1 : 1;
+
+                                    // If neither contain '?', parse the file 
sizes and compare.
+                                    long value1 = 
FileUtils.parseFileSize((String) e1.getValue().get(3));
+                                    long value2 = 
FileUtils.parseFileSize((String) e2.getValue().get(3));
+
+                                    return 
evaluateComparision(Long.compare(value1, value2));
+                                })
+                                .collect(toMap(Map.Entry::getKey, 
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+                    }
+                },
+        owns(true)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data.entrySet()
+                                .stream()
+                                .sorted((e1, e2) -> {
+                                    double value1 = 
Double.parseDouble(((String) e1.getValue().get(tokenPerNode ? 4 : 
5)).replace("%", ""));
+                                    double value2 = 
Double.parseDouble(((String) e2.getValue().get(tokenPerNode ? 4 : 
5)).replace("%", ""));
+
+                                    return 
evaluateComparision(Double.compare(value1, value2));
+                                })
+                                .collect(toMap(Map.Entry::getKey, 
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+                    }
+                },
+        id(false)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return compareStrings(data, tokenPerNode ? 5 : 6);
+                    }
+                },
+        rack(false)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return compareStrings(data, 7);
+                    }
+                },
+        state(true)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return compareStrings(data, 1);
+                    }
+                };
+
+        private final boolean descendingByDefault;
+        boolean tokenPerNode;
+        SortOrder sortOrder;
+
+        SortBy()
+        {
+            this(false);
+        }
+
+        SortBy(boolean descendingByDefault)
+        {
+            this.descendingByDefault = descendingByDefault;
+        }
+
+        public abstract Map<String, List<Object>> sort(Map<String, 
List<Object>> data);
+
+        boolean descending(SortOrder sortOrder)
+        {
+            return sortOrder == null ? descendingByDefault : sortOrder == 
SortOrder.desc;
+        }
+
+        SortBy sortOrder(SortOrder sortOrder)

Review Comment:
   should be marked private or protected



##########
src/java/org/apache/cassandra/tools/nodetool/Status.java:
##########
@@ -24,38 +25,232 @@
 import java.io.PrintStream;
 import java.net.UnknownHostException;
 import java.text.DecimalFormat;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.SortedMap;
 
+import org.apache.cassandra.io.util.FileUtils;
 import org.apache.cassandra.locator.EndpointSnitchInfoMBean;
+import org.apache.cassandra.locator.InetAddressAndPort;
 import org.apache.cassandra.tools.NodeProbe;
 import org.apache.cassandra.tools.NodeTool;
 import org.apache.cassandra.tools.NodeTool.NodeToolCmd;
 import org.apache.cassandra.tools.nodetool.formatter.TableBuilder;
 
 import com.google.common.collect.ArrayListMultimap;
 
+import static java.util.stream.Collectors.toMap;
+
 @SuppressWarnings("UseOfSystemOutOrSystemErr")
 @Command(name = "status", description = "Print cluster information (state, 
load, IDs, ...)")
-public class Status extends NodeToolCmd
-{
+public class Status extends NodeToolCmd {
     @Arguments(usage = "[<keyspace>]", description = "The keyspace name")
     private String keyspace = null;
 
     @Option(title = "resolve_ip", name = {"-r", "--resolve-ip"}, description = 
"Show node domain names instead of IPs")
     private boolean resolveIp = false;
 
+    @Option(title = "sort",
+            name = {"-s", "--sort"},
+            description = "Sort by one of 'ip', 'load', 'owns', 'id', 'rack' 
or 'state', Defaults to 'none', Default Ordering is 'asc' for 'ip', 'id', 
'rack' and 'desc' for 'load', 'owns', 'state' ",
+            allowedValues = {"ip", "load", "owns", "id", "rack", "state", 
"none"})
+    private SortBy sortBy = SortBy.none;
+
+    @Option(title = "sort_order",
+            name = {"-o", "--order"},
+            description = "Sort order: 'asc' for ascending, 'desc' for 
descending",
+            allowedValues = {"asc", "desc"})
+    private SortOrder sortOrder = null;
+
     private boolean isTokenPerNode = true;
     private Collection<String> joiningNodes, leavingNodes, movingNodes, 
liveNodes, unreachableNodes;
     private Map<String, String> loadMap, hostIDMap;
     private EndpointSnitchInfoMBean epSnitchInfo;
 
+    public enum SortOrder {
+        asc,
+        desc
+    }
+
+    public enum SortBy
+    {
+        none
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data;
+                    }
+                },
+        ip(false)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data.entrySet()
+                                .stream()
+                                .sorted((e1, e2) -> {
+                                    InetAddressAndPort addr1 = 
(InetAddressAndPort) e1.getValue().get(0);
+                                    InetAddressAndPort addr2 = 
(InetAddressAndPort) e2.getValue().get(0);
+
+                                    return 
evaluateComparision(addr1.compareTo(addr2));
+                                })
+                                .collect(toMap(Map.Entry::getKey, 
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+                    }
+                },
+        load(true)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data.entrySet()
+                                .stream()
+                                .sorted((e1, e2) -> {
+                                    String str1 = (String) 
e1.getValue().get(3);
+                                    String str2 = (String) 
e2.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 (containsQuestionMark1 && 
containsQuestionMark2) {
+                                        // If both contain '?', return 0 (they 
are considered equal).
+                                        return 0;
+                                    }
+
+                                    boolean descending = descending(sortOrder);
+
+                                    // If str1 contains '?', ensure it's last 
(or first depending on descending).
+                                    if (containsQuestionMark1)
+                                        return descending ? 1 : -1;
+
+                                    // If str2 contains '?', ensure it's last 
(or first depending on descending).
+                                    if (containsQuestionMark2)
+                                        return descending ? -1 : 1;
+
+                                    // If neither contain '?', parse the file 
sizes and compare.
+                                    long value1 = 
FileUtils.parseFileSize((String) e1.getValue().get(3));
+                                    long value2 = 
FileUtils.parseFileSize((String) e2.getValue().get(3));
+
+                                    return 
evaluateComparision(Long.compare(value1, value2));
+                                })
+                                .collect(toMap(Map.Entry::getKey, 
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+                    }
+                },
+        owns(true)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data.entrySet()
+                                .stream()
+                                .sorted((e1, e2) -> {
+                                    double value1 = 
Double.parseDouble(((String) e1.getValue().get(tokenPerNode ? 4 : 
5)).replace("%", ""));
+                                    double value2 = 
Double.parseDouble(((String) e2.getValue().get(tokenPerNode ? 4 : 
5)).replace("%", ""));
+
+                                    return 
evaluateComparision(Double.compare(value1, value2));
+                                })
+                                .collect(toMap(Map.Entry::getKey, 
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+                    }
+                },
+        id(false)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return compareStrings(data, tokenPerNode ? 5 : 6);
+                    }
+                },
+        rack(false)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return compareStrings(data, 7);
+                    }
+                },
+        state(true)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return compareStrings(data, 1);
+                    }
+                };
+
+        private final boolean descendingByDefault;
+        boolean tokenPerNode;
+        SortOrder sortOrder;
+
+        SortBy()
+        {
+            this(false);
+        }
+
+        SortBy(boolean descendingByDefault)
+        {
+            this.descendingByDefault = descendingByDefault;
+        }
+
+        public abstract Map<String, List<Object>> sort(Map<String, 
List<Object>> data);
+
+        boolean descending(SortOrder sortOrder)
+        {
+            return sortOrder == null ? descendingByDefault : sortOrder == 
SortOrder.desc;
+        }
+
+        SortBy sortOrder(SortOrder sortOrder)
+        {
+            this.sortOrder = sortOrder;
+            return this;
+        }
+
+        SortBy tokenPerNode(boolean tokenPerNode)
+        {
+            this.tokenPerNode = tokenPerNode;
+            return this;
+        }
+
+        int evaluateComparision(int comparisionResult)

Review Comment:
   should be marked protected



##########
src/java/org/apache/cassandra/tools/nodetool/Status.java:
##########
@@ -24,38 +25,232 @@
 import java.io.PrintStream;
 import java.net.UnknownHostException;
 import java.text.DecimalFormat;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.SortedMap;
 
+import org.apache.cassandra.io.util.FileUtils;
 import org.apache.cassandra.locator.EndpointSnitchInfoMBean;
+import org.apache.cassandra.locator.InetAddressAndPort;
 import org.apache.cassandra.tools.NodeProbe;
 import org.apache.cassandra.tools.NodeTool;
 import org.apache.cassandra.tools.NodeTool.NodeToolCmd;
 import org.apache.cassandra.tools.nodetool.formatter.TableBuilder;
 
 import com.google.common.collect.ArrayListMultimap;
 
+import static java.util.stream.Collectors.toMap;
+
 @SuppressWarnings("UseOfSystemOutOrSystemErr")
 @Command(name = "status", description = "Print cluster information (state, 
load, IDs, ...)")
-public class Status extends NodeToolCmd
-{
+public class Status extends NodeToolCmd {
     @Arguments(usage = "[<keyspace>]", description = "The keyspace name")
     private String keyspace = null;
 
     @Option(title = "resolve_ip", name = {"-r", "--resolve-ip"}, description = 
"Show node domain names instead of IPs")
     private boolean resolveIp = false;
 
+    @Option(title = "sort",
+            name = {"-s", "--sort"},
+            description = "Sort by one of 'ip', 'load', 'owns', 'id', 'rack' 
or 'state', Defaults to 'none', Default Ordering is 'asc' for 'ip', 'id', 
'rack' and 'desc' for 'load', 'owns', 'state' ",
+            allowedValues = {"ip", "load", "owns", "id", "rack", "state", 
"none"})
+    private SortBy sortBy = SortBy.none;
+
+    @Option(title = "sort_order",
+            name = {"-o", "--order"},
+            description = "Sort order: 'asc' for ascending, 'desc' for 
descending",
+            allowedValues = {"asc", "desc"})
+    private SortOrder sortOrder = null;
+
     private boolean isTokenPerNode = true;
     private Collection<String> joiningNodes, leavingNodes, movingNodes, 
liveNodes, unreachableNodes;
     private Map<String, String> loadMap, hostIDMap;
     private EndpointSnitchInfoMBean epSnitchInfo;
 
+    public enum SortOrder {
+        asc,
+        desc
+    }
+
+    public enum SortBy
+    {
+        none
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data;
+                    }
+                },
+        ip(false)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data.entrySet()
+                                .stream()
+                                .sorted((e1, e2) -> {
+                                    InetAddressAndPort addr1 = 
(InetAddressAndPort) e1.getValue().get(0);
+                                    InetAddressAndPort addr2 = 
(InetAddressAndPort) e2.getValue().get(0);
+
+                                    return 
evaluateComparision(addr1.compareTo(addr2));
+                                })
+                                .collect(toMap(Map.Entry::getKey, 
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+                    }
+                },
+        load(true)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data.entrySet()
+                                .stream()
+                                .sorted((e1, e2) -> {
+                                    String str1 = (String) 
e1.getValue().get(3);
+                                    String str2 = (String) 
e2.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 (containsQuestionMark1 && 
containsQuestionMark2) {
+                                        // If both contain '?', return 0 (they 
are considered equal).
+                                        return 0;
+                                    }
+
+                                    boolean descending = descending(sortOrder);
+
+                                    // If str1 contains '?', ensure it's last 
(or first depending on descending).
+                                    if (containsQuestionMark1)
+                                        return descending ? 1 : -1;
+
+                                    // If str2 contains '?', ensure it's last 
(or first depending on descending).
+                                    if (containsQuestionMark2)
+                                        return descending ? -1 : 1;
+
+                                    // If neither contain '?', parse the file 
sizes and compare.
+                                    long value1 = 
FileUtils.parseFileSize((String) e1.getValue().get(3));
+                                    long value2 = 
FileUtils.parseFileSize((String) e2.getValue().get(3));
+
+                                    return 
evaluateComparision(Long.compare(value1, value2));
+                                })
+                                .collect(toMap(Map.Entry::getKey, 
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+                    }
+                },
+        owns(true)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data.entrySet()
+                                .stream()
+                                .sorted((e1, e2) -> {
+                                    double value1 = 
Double.parseDouble(((String) e1.getValue().get(tokenPerNode ? 4 : 
5)).replace("%", ""));
+                                    double value2 = 
Double.parseDouble(((String) e2.getValue().get(tokenPerNode ? 4 : 
5)).replace("%", ""));
+
+                                    return 
evaluateComparision(Double.compare(value1, value2));
+                                })
+                                .collect(toMap(Map.Entry::getKey, 
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+                    }
+                },
+        id(false)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return compareStrings(data, tokenPerNode ? 5 : 6);
+                    }
+                },
+        rack(false)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return compareStrings(data, 7);
+                    }
+                },
+        state(true)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return compareStrings(data, 1);
+                    }
+                };
+
+        private final boolean descendingByDefault;
+        boolean tokenPerNode;
+        SortOrder sortOrder;
+
+        SortBy()
+        {
+            this(false);
+        }
+
+        SortBy(boolean descendingByDefault)
+        {
+            this.descendingByDefault = descendingByDefault;
+        }
+
+        public abstract Map<String, List<Object>> sort(Map<String, 
List<Object>> data);
+
+        boolean descending(SortOrder sortOrder)
+        {
+            return sortOrder == null ? descendingByDefault : sortOrder == 
SortOrder.desc;
+        }
+
+        SortBy sortOrder(SortOrder sortOrder)
+        {
+            this.sortOrder = sortOrder;
+            return this;
+        }
+
+        SortBy tokenPerNode(boolean tokenPerNode)
+        {
+            this.tokenPerNode = tokenPerNode;
+            return this;
+        }
+
+        int evaluateComparision(int comparisionResult)
+        {
+            if (comparisionResult < 0)
+                return descending(sortOrder) ? 1 : -1;
+            if (comparisionResult > 0)
+                return descending(sortOrder) ? -1 : 1;
+
+            return 0;
+        }
+
+        LinkedHashMap<String, List<Object>> compareStrings(Map<String, 
List<Object>> data, int index)
+        {
+            return data.entrySet()
+                    .stream()
+                    .sorted((e1, e2) -> {
+                        String str1 = (String) e1.getValue().get(index);
+                        String str2 = (String) e2.getValue().get(index);
+                        return evaluateComparision(str1.compareTo(str2));
+                    })
+                    .collect(toMap(Map.Entry::getKey, Map.Entry::getValue, 
(e1, e2) -> e1, LinkedHashMap::new));
+        }
+    }
+
+    private void validateArguments(PrintStream out)

Review Comment:
   minor nit: I find this cleaner after `execute` since thats the function 
folks look for first when they read the code



##########
src/java/org/apache/cassandra/tools/nodetool/Status.java:
##########
@@ -24,38 +25,232 @@
 import java.io.PrintStream;
 import java.net.UnknownHostException;
 import java.text.DecimalFormat;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.SortedMap;
 
+import org.apache.cassandra.io.util.FileUtils;
 import org.apache.cassandra.locator.EndpointSnitchInfoMBean;
+import org.apache.cassandra.locator.InetAddressAndPort;
 import org.apache.cassandra.tools.NodeProbe;
 import org.apache.cassandra.tools.NodeTool;
 import org.apache.cassandra.tools.NodeTool.NodeToolCmd;
 import org.apache.cassandra.tools.nodetool.formatter.TableBuilder;
 
 import com.google.common.collect.ArrayListMultimap;
 
+import static java.util.stream.Collectors.toMap;
+
 @SuppressWarnings("UseOfSystemOutOrSystemErr")
 @Command(name = "status", description = "Print cluster information (state, 
load, IDs, ...)")
-public class Status extends NodeToolCmd
-{
+public class Status extends NodeToolCmd {
     @Arguments(usage = "[<keyspace>]", description = "The keyspace name")
     private String keyspace = null;
 
     @Option(title = "resolve_ip", name = {"-r", "--resolve-ip"}, description = 
"Show node domain names instead of IPs")
     private boolean resolveIp = false;
 
+    @Option(title = "sort",
+            name = {"-s", "--sort"},
+            description = "Sort by one of 'ip', 'load', 'owns', 'id', 'rack' 
or 'state', Defaults to 'none', Default Ordering is 'asc' for 'ip', 'id', 
'rack' and 'desc' for 'load', 'owns', 'state' ",
+            allowedValues = {"ip", "load", "owns", "id", "rack", "state", 
"none"})
+    private SortBy sortBy = SortBy.none;
+
+    @Option(title = "sort_order",
+            name = {"-o", "--order"},
+            description = "Sort order: 'asc' for ascending, 'desc' for 
descending",
+            allowedValues = {"asc", "desc"})
+    private SortOrder sortOrder = null;
+
     private boolean isTokenPerNode = true;
     private Collection<String> joiningNodes, leavingNodes, movingNodes, 
liveNodes, unreachableNodes;
     private Map<String, String> loadMap, hostIDMap;
     private EndpointSnitchInfoMBean epSnitchInfo;
 
+    public enum SortOrder {
+        asc,
+        desc
+    }
+
+    public enum SortBy
+    {
+        none
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data;
+                    }
+                },
+        ip(false)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data.entrySet()
+                                .stream()
+                                .sorted((e1, e2) -> {
+                                    InetAddressAndPort addr1 = 
(InetAddressAndPort) e1.getValue().get(0);
+                                    InetAddressAndPort addr2 = 
(InetAddressAndPort) e2.getValue().get(0);
+
+                                    return 
evaluateComparision(addr1.compareTo(addr2));
+                                })
+                                .collect(toMap(Map.Entry::getKey, 
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+                    }
+                },
+        load(true)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data.entrySet()
+                                .stream()
+                                .sorted((e1, e2) -> {
+                                    String str1 = (String) 
e1.getValue().get(3);
+                                    String str2 = (String) 
e2.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 (containsQuestionMark1 && 
containsQuestionMark2) {
+                                        // If both contain '?', return 0 (they 
are considered equal).
+                                        return 0;
+                                    }
+
+                                    boolean descending = descending(sortOrder);
+
+                                    // If str1 contains '?', ensure it's last 
(or first depending on descending).
+                                    if (containsQuestionMark1)
+                                        return descending ? 1 : -1;
+
+                                    // If str2 contains '?', ensure it's last 
(or first depending on descending).
+                                    if (containsQuestionMark2)
+                                        return descending ? -1 : 1;
+
+                                    // If neither contain '?', parse the file 
sizes and compare.
+                                    long value1 = 
FileUtils.parseFileSize((String) e1.getValue().get(3));
+                                    long value2 = 
FileUtils.parseFileSize((String) e2.getValue().get(3));
+
+                                    return 
evaluateComparision(Long.compare(value1, value2));
+                                })
+                                .collect(toMap(Map.Entry::getKey, 
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+                    }
+                },
+        owns(true)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return data.entrySet()
+                                .stream()
+                                .sorted((e1, e2) -> {
+                                    double value1 = 
Double.parseDouble(((String) e1.getValue().get(tokenPerNode ? 4 : 
5)).replace("%", ""));
+                                    double value2 = 
Double.parseDouble(((String) e2.getValue().get(tokenPerNode ? 4 : 
5)).replace("%", ""));
+
+                                    return 
evaluateComparision(Double.compare(value1, value2));
+                                })
+                                .collect(toMap(Map.Entry::getKey, 
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+                    }
+                },
+        id(false)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return compareStrings(data, tokenPerNode ? 5 : 6);
+                    }
+                },
+        rack(false)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return compareStrings(data, 7);
+                    }
+                },
+        state(true)
+                {
+                    @Override
+                    public Map<String, List<Object>> sort(Map<String, 
List<Object>> data)
+                    {
+                        return compareStrings(data, 1);
+                    }
+                };
+
+        private final boolean descendingByDefault;
+        boolean tokenPerNode;
+        SortOrder sortOrder;
+
+        SortBy()
+        {
+            this(false);
+        }
+
+        SortBy(boolean descendingByDefault)
+        {
+            this.descendingByDefault = descendingByDefault;
+        }
+
+        public abstract Map<String, List<Object>> sort(Map<String, 
List<Object>> data);
+
+        boolean descending(SortOrder sortOrder)
+        {
+            return sortOrder == null ? descendingByDefault : sortOrder == 
SortOrder.desc;
+        }
+
+        SortBy sortOrder(SortOrder sortOrder)
+        {
+            this.sortOrder = sortOrder;
+            return this;
+        }
+
+        SortBy tokenPerNode(boolean tokenPerNode)
+        {
+            this.tokenPerNode = tokenPerNode;
+            return this;
+        }
+
+        int evaluateComparision(int comparisionResult)
+        {
+            if (comparisionResult < 0)
+                return descending(sortOrder) ? 1 : -1;
+            if (comparisionResult > 0)
+                return descending(sortOrder) ? -1 : 1;
+
+            return 0;
+        }
+
+        LinkedHashMap<String, List<Object>> compareStrings(Map<String, 
List<Object>> data, int index)

Review Comment:
   should be marked protected



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to