Update range function to follow Python's range function

main/src/com/google/refine/expr/functions/strings/Range.java

@@ -4,7 +4,6 @@ import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.Properties;
 
-import org.apache.commons.lang3.StringUtils;
 import org.json.JSONException;
 import org.json.JSONWriter;
 
@@ -16,32 +15,31 @@ import com.google.refine.grel.Function;
  * Implements the logic behind the range function.
  * 
  * The range function can take in a single string of the form 'a, b, c' or three
- * integers a, b, c where a and b represents the first and last numbers in the range respectively.
+ * integers a, b, c where a and b represents the first (inclusive) and last (exclusive)
- * 
+ * numbers in the range respectively. If b is not given, a defaults to the range end 
- * If b is not given, a defaults to the range end and 0 becomes the range start.
+ * and 0 becomes the range start. c is optional and represents the step (increment) 
- * c is optional and represents the step (increment) for the generated sequence.
+ * for the generated sequence.
  */
 public class Range implements Function {
 
-    private static final int STRING_ARG_LENGTH = 1;
     private static final String SEPARATOR = ",";
 
-    private static final int INTEGER_ARGS_LENGTH = 2;
-    private static final int INTEGER_ARGS_WITH_STEP = 3;
-    
-    private static final String DEFAULT_RANGE_START = "0";
-    
     private static final String lastCharacterCommaRegex = ",$";
     private static final Pattern lastCharacterCommaPattern = Pattern.compile(lastCharacterCommaRegex);
 
+    private static final int DEFAULT_START = 0;
+    private static final int DEFAULT_STEP = 1;
+
+    private static final String[] EMPTY_STRING_ARRAY = new String[0];
+
     @Override
     public Object call(Properties bindings, Object[] args) {
 
-        if (args.length == STRING_ARG_LENGTH) {
+        if (args.length == 1) {
             return createRangeWithOneGivenArgument(args);
-        } else if (args.length == INTEGER_ARGS_LENGTH) {
+        } else if (args.length == 2) {
             return createRangeWithTwoGivenArguments(args);
-        } else if (args.length == INTEGER_ARGS_WITH_STEP) {
+        } else if (args.length == 3) {
             return createRangeWithThreeGivenArguments(args);
         }
 
@@ -53,7 +51,7 @@ public class Range implements Function {
     /**
      * Checks if a given string has a comma as the last character.
      * 
-     * This is primarily used against abusing the range function like doing range("1,").
+     * This is primarily used to detect edge cases like doing range("1,").
      */
     private boolean hasCommaAsLastCharacter(String test) {
         Matcher lastCharacterCommaMatcher = lastCharacterCommaPattern.matcher(test);
@@ -61,52 +59,72 @@ public class Range implements Function {
     }
 
     /**
-     * Processes the single argument given to determine if the argument is a valid string, a 
+     * Processes the single argument given to determine if the argument is (i) a valid string, (ii) a 
-     * valid integer, or an invalid argument.
+     * valid integer, or (iii) an invalid argument.
      * 
      * If the argument is a valid string, it can either be in the form 'a', or 'a, b' or 'a, b, c' 
      * where a and b are the start and end of the range respectively, and c is the optional
-     * step argument. In the case where 'a' is the only argument, 'a' becomes the range end and 
+     * step argument. In the case where 'a' is the only argument, 'a' becomes the range end (exclusive)
-     * 0 becomes the default range start.
+     * and 0 becomes the default range start.
      * 
      * If the argument is a valid integer, it can will default to become the range end, and 0 defaults
      * to become the range start.
+     * 
+     * In all other cases, the argument is considered invalid.
      */
     private Object createRangeWithOneGivenArgument(Object[] args) {
         Object range = args[0];
 
+        int rangeStart = DEFAULT_START;
+        int rangeEnd = 0;
+        int rangeStep = DEFAULT_STEP;
+
+        // Check for valid string argument(s)
         if (range != null && range instanceof String) {
             String rangeString = ((String) range).trim();
             String[] rangeValues = rangeString.split(SEPARATOR);
 
-            if (rangeValues.length == 1 && StringUtils.isNumeric(rangeValues[0])
+            if (hasCommaAsLastCharacter(rangeString)) {
-                    && !hasCommaAsLastCharacter(rangeString)) {
+                return new EvalError("the last character in the input string should not be a comma");
-                String rangeEnd = rangeValues[0].trim();
-
-                return createRange(DEFAULT_RANGE_START, rangeEnd);
-            } else if (rangeValues.length == INTEGER_ARGS_LENGTH) {
-                String rangeStart = rangeValues[0].trim();
-                String rangeEnd = rangeValues[1].trim();
-
-                if (StringUtils.isNumeric(rangeStart) && StringUtils.isNumeric(rangeEnd)) {
-                    return createRange(rangeStart, rangeEnd);
             }
-            } else if (rangeValues.length == INTEGER_ARGS_WITH_STEP) {
-                String rangeStart = rangeValues[0].trim();
-                String rangeEnd = rangeValues[1].trim();
-                String rangeStep = rangeValues[2].trim();
 
-                if (StringUtils.isNumeric(rangeStart) && StringUtils.isNumeric(rangeEnd) 
+            try {
-                        && StringUtils.isNumeric(rangeStep)) {
+                if (rangeValues.length == 1) {
+                    rangeEnd = Integer.parseInt(rangeValues[0].trim());
+                    return createRange(rangeStart, rangeEnd, rangeStep);
+                } else if (rangeValues.length == 2) {
+                    rangeStart = Integer.parseInt(rangeValues[0].trim());
+                    rangeEnd = Integer.parseInt(rangeValues[1].trim());
+                    return createRange(rangeStart, rangeEnd, rangeStep);
+                } else if (rangeValues.length == 3) {
+                    rangeStart = Integer.parseInt(rangeValues[0].trim());
+                    rangeEnd = Integer.parseInt(rangeValues[1].trim());
+                    rangeStep = Integer.parseInt(rangeValues[2].trim());
                     return createRange(rangeStart, rangeEnd, rangeStep);
                 }
+            } catch (NumberFormatException nfe) {
+                return new EvalError(ControlFunctionRegistry.getFunctionName(this) 
+                        + " expects a string of the form 'a, b, c' or integers a, b, c where a and b "
+                        + "are the start and the end of the range respectively and c is the step (increment)");
             }
         }
 
-        if (range != null && StringUtils.isNumeric(range.toString())) {
+        // Check for valid negative integer argument
-            String rangeEnd = range.toString();
+        if (range != null && range instanceof Double && (Double) range % 1 == 0) {
+            range = ((Double) range).intValue();
+            return createRange(DEFAULT_START, rangeEnd, DEFAULT_STEP);
+        }
 
-            return createRange(DEFAULT_RANGE_START, rangeEnd);
+        // Check for valid positive integer argument
+        if (range != null) {
+            try {
+                rangeEnd = Integer.parseInt(String.valueOf(range));
+                return createRange(DEFAULT_START, rangeEnd, DEFAULT_STEP);
+            } catch (NumberFormatException nfe) {
+                return new EvalError(ControlFunctionRegistry.getFunctionName(this) 
+                        + " expects a string of the form 'a, b, c' or integers a, b, c where a and b "
+                        + "are the start and the end of the range respectively and c is the step (increment)");
+            }
         }
 
         return new EvalError(ControlFunctionRegistry.getFunctionName(this) 
@@ -115,38 +133,101 @@ public class Range implements Function {
     }
 
     /**
-     * Processes the two arguments given to determine if the arguments are two valid integers or 
+     * Processes the two arguments given to determine if the arguments are (i) two valid strings, 
-     * a valid string and step or invalid arguments.
+     * (ii) two valid integers or (iii) a valid string and an integer or (iv) invalid arguments.
+     * 
+     * If the arguments are valid strings, the strings can either be such that (i) each string contains 
+     * single arguments (i.e. two arguments in total), or (ii) one string contains one argument and the other 
+     * string contains two argument (i.e. three arguments in total).
+     * 
+     * If the arguments are a valid string and a valid integer, the string can be such that (i) the string
+     * contains a single argument (i.e. two arguments in total) or (ii) the string contains two arguments 
+     * (i.e. three arguments in total).
+     * 
+     * In all other cases, the arguments are considered invalid.
      */
     private Object createRangeWithTwoGivenArguments(Object[] args) {
-        String rangeStart = args[0].toString().trim();
+        Object firstArg = args[0];
-        String rangeEnd = args[1].toString().trim();
+        Object secondArg = args[1];
 
-        String range = rangeStart;
+        int rangeStart = DEFAULT_START;
-        String rangeStep = rangeEnd;
+        int rangeEnd = 0;
+        int rangeStep = DEFAULT_STEP;
 
-        boolean isTwoValidIntegers = false;
+        if (firstArg == null || secondArg == null) {
-        boolean isValidStringWithStep = false;
+            return new EvalError(ControlFunctionRegistry.getFunctionName(this) 
-        
+                    + " expects a string of the form 'a, b, c' or integers a, b, c where a and b "
-        String[] rangeValues = range.split(SEPARATOR);
+                    + "are the start and the end of the range respectively and c is the step (increment)");
-        
-        if (rangeValues.length == 1) {
-            isTwoValidIntegers = true;
-        } else if (rangeValues.length == 2) {
-            isValidStringWithStep = true;
         }
 
-        if (isTwoValidIntegers && StringUtils.isNumeric(rangeStart) &&
+        boolean hasString = false;
-                StringUtils.isNumeric(rangeEnd)) {
+        boolean hasTwoIntegers = true;
-            return createRange(rangeStart, rangeEnd);
-        } else if (isValidStringWithStep) {
-            rangeStart = rangeValues[0].trim();
-            rangeEnd = rangeValues[1].trim();
 
-            if (StringUtils.isNumeric(rangeStart) && StringUtils.isNumeric(rangeEnd) 
+        if (firstArg instanceof String || secondArg instanceof String) {
-                    && StringUtils.isNumeric(rangeStep)) {
+            hasString = true;
+            hasTwoIntegers = false;
+        }
+
+        boolean hasTwoArguments = hasTwoIntegers;
+        boolean hasThreeArguments = false;
+
+        // Deal with valid negative integers
+        if (firstArg instanceof Double && (Double) firstArg % 1 == 0) {
+            firstArg = ((Double) firstArg).intValue();
+        }
+
+        if (secondArg instanceof Double && (Double) secondArg % 1 == 0) {
+            secondArg = ((Double) secondArg).intValue();
+        }
+
+        String firstArgStringified = String.valueOf(firstArg).trim();
+        String secondArgStringified = String.valueOf(secondArg).trim();
+        String thirdArgStringified = "";
+
+        if (hasCommaAsLastCharacter(firstArgStringified) || hasCommaAsLastCharacter(secondArgStringified)) {
+            return new EvalError("the last character in the input string should not be a comma");
+        }
+
+        // Check if the strings are valid strings (e.g. range("1, 2", "3, 4") should fail but 
+        // range("1, 2", "1") should pass)
+        if (hasString) {            
+            String[] firstArgArray = firstArgStringified.split(SEPARATOR);
+            String[] secondArgArray = secondArgStringified.split(SEPARATOR);
+
+            int combinedArrayLength = firstArgArray.length + secondArgArray.length;
+
+            if (combinedArrayLength == 3) {
+                hasThreeArguments = true;
+ 
+                if (firstArgArray.length == 1) {
+                    secondArgStringified = secondArgArray[0].trim();
+                    thirdArgStringified = secondArgArray[1].trim();
+                } else {
+                    firstArgStringified = firstArgArray[0].trim();
+                    secondArgStringified = firstArgArray[1].trim();
+                    thirdArgStringified = secondArgArray[0].trim();
+                }
+
+            } else if (combinedArrayLength == 2) {
+                hasTwoArguments = true;
+            }
+        }
+
+        try {
+            if (hasTwoArguments) {
+                rangeStart = Integer.parseInt(firstArgStringified);
+                rangeEnd = Integer.parseInt(secondArgStringified);
+                return createRange(rangeStart, rangeEnd, rangeStep);
+            } else if (hasThreeArguments) {
+                rangeStart = Integer.parseInt(firstArgStringified);
+                rangeEnd = Integer.parseInt(secondArgStringified);
+                rangeStep = Integer.parseInt(thirdArgStringified);
                 return createRange(rangeStart, rangeEnd, rangeStep);
             }
+        } catch (NumberFormatException nfe) {
+            return new EvalError(ControlFunctionRegistry.getFunctionName(this) 
+                    + " expects a string of the form 'a, b, c' or integers a, b, c where a and b "
+                    + "are the start and the end of the range respectively and c is the step (increment)");   
         }
 
         return new EvalError(ControlFunctionRegistry.getFunctionName(this) 
@@ -155,31 +236,40 @@ public class Range implements Function {
     }
 
     /**
-     * Processes the three arguments given to determine if the arguments are three valid integers or 
+     * Processes the three arguments given to determine if the arguments are (i) three valid strings, 
-     * invalid arguments.
+     * (ii) three valid integers, (iii) two valid strings and a valid integer, (iv) a valid string and 
+     * two valid integers or (v) invalid arguments.
+     * 
+     * In this case, all valid strings can only contain a single argument.
      */
     private Object createRangeWithThreeGivenArguments(Object[] args) {
-        String rangeStart = args[0].toString().trim();
+        Object firstArg = args[0];
-        String rangeEnd = args[1].toString().trim();
+        Object secondArg = args[1];
-        String rangeStep = args[2].toString().trim();
+        Object thirdArg = args[2];
-        if (StringUtils.isNumeric(rangeStart) && StringUtils.isNumeric(rangeEnd) 
+      
-                && StringUtils.isNumeric(rangeStep)) {
+        // Deal with negative integers first
-            return createRange(rangeStart, rangeEnd, rangeStep);
+        if (firstArg != null && firstArg instanceof Double && (Double) firstArg % 1 == 0) {
+            firstArg = ((Double) firstArg).intValue();
         }
 
+        if (secondArg != null && secondArg instanceof Double && (Double) secondArg % 1 == 0) {
+            secondArg = ((Double) secondArg).intValue();
+        }
+
+        if (thirdArg != null && thirdArg instanceof Double && (Double) thirdArg % 1 == 0) {
+            thirdArg = ((Double) thirdArg).intValue();
+        }
+
+        try {    
+            int rangeStart = Integer.parseInt(String.valueOf(firstArg).trim());
+            int rangeEnd = Integer.parseInt(String.valueOf(secondArg).trim());
+            int rangeStep = Integer.parseInt(String.valueOf(thirdArg).trim());
+            return createRange(rangeStart, rangeEnd, rangeStep);
+        } catch (NumberFormatException nfe) {
             return new EvalError(ControlFunctionRegistry.getFunctionName(this) 
                     + " expects a string of the form 'a, b, c' or integers a, b, c where a and b "
                     + "are the start and the end of the range respectively and c is the step (increment)");
         }
-    
-    /**
-     * Creates a range from the given range values.
-     * 
-     * The generated range is either an increasing sequence or a decreasing sequence, and 
-     * each number in the sequence differs from the next number by one.
-     */
-    private static Object createRange(String rangeStart, String rangeEnd) {
-        return createRange(rangeStart, rangeEnd, "1"); 
     }
 
     /**
@@ -188,28 +278,18 @@ public class Range implements Function {
      * The generated range is either an increasing sequence or a decreasing sequence, and 
      * each number in the sequence differs from the next number by the step value. 
      */
-    private static Object createRange(String rangeStart, String rangeEnd, String rangeStep) {
+    private static Object createRange(int start, int stop, int step) {        
-        int start = Integer.parseInt(rangeStart);
+        if ((start > stop && step > 0) || (start < stop && step < 0) || step == 0) {
-        int end = Integer.parseInt(rangeEnd);
+            return EMPTY_STRING_ARRAY;
-        int step = Integer.parseInt(rangeStep);
-        int negativeStep = -step;        
-        int rangeSize = 0;
-        
-        if (step != 0) {
-            rangeSize = (int) (Math.ceil((double) (Math.abs(start - end) + 1)/ step));
         }
 
+        int rangeSize = (int) (Math.ceil(((double) Math.abs(start - stop))/ Math.abs(step)));
+
         String[] generatedRange = new String[rangeSize];
 
-        if (start < end) {
         for (int i = 0; i < rangeSize; i++) {
            generatedRange[i] = Integer.toString(start + step * i);
         }
-        } else {
-            for (int i = 0; i < rangeSize; i++) {
-                generatedRange[i] = Integer.toString(start + negativeStep * i);
-            }
-        }
 
         return generatedRange;   
     }