Github user paulk-asert commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/360#discussion_r70180723
  
    --- Diff: src/main/groovy/lang/ObjectRange.java ---
    @@ -493,40 +495,52 @@ public void remove() {
     
         /**
          * Increments by step size
    +     * <p/>
    +     * It uses arithmetic plus for {@link Number} instances and the 
repeated `next` method for other {@link Comparable}s.
          *
          * @param value the value to increment
          * @param step  the value to increment by
          * @return the incremented value or null, if there isn't any
          */
         @SuppressWarnings("unchecked")
         private T increment(T value, int step) {
    -        for (int i = 0; i < step; i++) {
    -            final T next = (T) InvokerHelper.invokeMethod(value, "next", 
null);
    -            if (!compareGreaterThan(next, value)) /* e.g. `next` of the 
last element */ {
    -                return null;
    +        if (value instanceof Number) {
    +            return (T) plus((Number) value, step);
    +        } else {
    +            for (int i = 0; i < step; i++) {
    +                final T next = (T) InvokerHelper.invokeMethod(value, 
"next", null);
    +                if (!compareGreaterThan(next, value)) /* e.g. `next` of 
the last element */ {
    +                    return null;
    +                }
    +                value = next;
                 }
    -            value = next;
    +            return value;
             }
    -        return value;
         }
     
         /**
          * Decrements by step size
    +     * <p/>
    +     * It uses arithmetic minus for {@link Number} instances and the 
repeated `previous` method for other {@link Comparable}s.
          *
          * @param value the value to decrement
          * @param step  the value to decrement by
          * @return the decremented value or null, if there isn't any
          */
         @SuppressWarnings("unchecked")
         private T decrement(T value, int step) {
    -        for (int i = 0; i < step; i++) {
    -            final T previous = (T) InvokerHelper.invokeMethod(value, 
"previous", null);
    -            if (!compareLessThan(previous, value)) /* e.g. `previous` of 
the first element */ {
    -                return null;
    +        if (value instanceof Number) {
    +            return (T) minus((Number) value, step);
    +        } else {
    +            for (int i = 0; i < step; i++) {
    +                final T previous = (T) InvokerHelper.invokeMethod(value, 
"previous", null);
    +                if (!compareLessThan(previous, value)) /* e.g. `previous` 
of the first element */ {
    +                    return null;
    +                }
    +                value = previous;
                 }
    --- End diff --
    
    So we still need to decide if we want this breaking change and whether 
javadoc on a private method (which won't be included in the generated javadoc) 
is sufficient to advertise the change and as stated previously consider whether 
to have a compatibility story.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to