[GitHub] [tinkerpop] robertdale commented on a change in pull request #1120: gt/gte/lt/lte can throw CCE if object isn't a Comparable

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

robertdale commented on a change in pull request #1120: gt/gte/lt/lte can throw CCE if object isn't a Comparable
URL: https://github.com/apache/tinkerpop/pull/1120#discussion_r288947051
 
 

 ##########
 File path: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Compare.java
 ##########
 @@ -167,6 +181,20 @@ public Compare negate() {
         }
     };
 
+    private static boolean bothAreNumber(Object first, Object second) {
+        return first instanceof Number && second instanceof Number;
+    }
+
+    private static boolean bothAreComparable(Object first, Object second) {
+        return first instanceof Comparable && second instanceof Comparable;
+    }
+
+    private static void checkBothAreNumberOrComparable(Object first, Object second) {
+        if (null != first && null != second && !bothAreNumber(first, second) && !bothAreComparable(first, second)) {
+            throw new IllegalArgumentException(String.format("'%s' and '%s' need to be an instance of Number or Comparable", first, second));
 
 Review comment:
   You could do away with the double checking too...
   ```
      if (null != first && null != second) {
         if (bothAreNumber(first, second)) {
            return NumberHelper.compare((Number) first, (Number) second) < 0;
         } else if (bothAreComparable(first, second)) {
            return ((Comparable) first).compareTo(second) < 0;
         } else {
            throw new IllegalArgumentException(String.format("Cannot compare '%s' (%s) and '%s' (%s) as both need to be an instance of Number or Comparable", first, first.getClass().getSimpleName(), second, second.getClass().getSimpleName()));
         }
      } else 
         return false;
   ```

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


With regards,
Apache Git Services