You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@groovy.apache.org by Paul King <pa...@asert.com.au> on 2024/01/10 04:53:47 UTC

Possible improvement to NumberRange

Hi folks,

The NumberRange abstraction tries very hard to allow any Number
numeric type to be used but since the Number interface doesn't convey
much behavior, there are a few places where it defaults to using
Groovy's NumberMath plumbing which, to cut a long story short, falls
back to using BigDecimal for any numeric calculations which aren't
using the common known simpler types.

A consequence of this is that currently if you created a range using
e.g. the Apache Commons Fraction class (which does extend Number), and
used a Fraction stepSize, the values in the range would be one
Fraction (for the first element) and then subsequent elements would be
BigDecimals.

@Grab('org.apache.commons:commons-lang3:3.14.0')
import org.apache.commons.lang3.math.Fraction
def r = (Fraction.ONE..2).by(Fraction.ONE_QUARTER)
println r.toList() // => [1/1, 1.25, 1.50, 1.75, 2.00]

This isn't incorrect in one sense but is somewhat surprising. Given
that the Number interface doesn't have operators, providing a smarter
detection of the number system to use becomes somewhat tricky. One
thing we could do is provide some interface that providers could use
and we could have a "Fraction" math implementation that satisfied that
interface. Alternatively, we could supply some [Bi]Functions that
offered the supplied behavior that the StepIterator needs when
calculating subsequent elements in the range. With this second
approach, we could do something like:

@Grab('org.apache.commons:commons-lang3:3.14.0')
import org.apache.commons.lang3.math.Fraction
(Fraction.ONE..2).by(Fraction.ONE_QUARTER,
    Fraction::add, Fraction::subtract, Fraction::negate).toList()

Which gives a list of all Fraction instances: [1/1, 5/4, 3/2, 7/4, 2/1]

Is this something we should support? Does anyone have ideas on the
best implementation?

Patch of a prototype is provided below. I can turn into a PR with tests
but I am trying to gauge what folks think first.

Thoughts? Paul.

=========== >8 =============

Subject: [PATCH] NumberRangeTweaks
---
Index: src/main/java/groovy/lang/NumberRange.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/main/java/groovy/lang/NumberRange.java
b/src/main/java/groovy/lang/NumberRange.java
--- a/src/main/java/groovy/lang/NumberRange.java (revision
3cd76364f772250324f5729ef93ffd76fbdd2b79)
+++ b/src/main/java/groovy/lang/NumberRange.java (date 1704856055407)
@@ -31,6 +31,8 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.NoSuchElementException;
+import java.util.function.BiFunction;
+import java.util.function.Function;

 import static org.codehaus.groovy.runtime.ScriptBytecodeAdapter.compareEqual;
 import static org.codehaus.groovy.runtime.ScriptBytecodeAdapter.compareGreaterThan;
@@ -94,6 +96,9 @@
      * <code>true</code> if the range includes the upper bound.
      */
     private final boolean inclusiveRight;
+    private BiFunction<Number, Number, Number> increment = null;
+    private BiFunction<Number, Number, Number> decrement = null;
+    private Function<Number, Number> negate = null;

     /**
      * Creates an inclusive {@link NumberRange} with step size 1.
@@ -246,6 +251,17 @@
         return new NumberRange(comparableNumber(from),
comparableNumber(to), stepSize, inclusiveLeft, inclusiveRight);
     }

+    public <T extends Number & Comparable> NumberRange by(T stepSize,
BiFunction<Number, Number, Number> increment, BiFunction<Number,
Number, Number> decrement, Function<Number, Number> negate) {
+        if (!Integer.valueOf(1).equals(this.stepSize)) {
+            throw new IllegalStateException("by only allowed on
ranges with original stepSize = 1 but found " + this.stepSize);
+        }
+        NumberRange result = new NumberRange(comparableNumber(from),
comparableNumber(to), stepSize, inclusiveLeft, inclusiveRight);
+        result.increment = increment;
+        result.decrement = decrement;
+        result.negate = negate;
+        return result;
+    }
+
     @SuppressWarnings("unchecked")
     /* package private */ static <T extends Number & Comparable> T
comparableNumber(Comparable c) {
         return (T) c;
@@ -617,7 +633,7 @@

             this.range = range;
             if (compareLessThan(step, 0)) {
-                this.step = multiply(step, -1);
+                this.step = negate != null ? negate.apply(step) :
multiply(step, -1);
                 isAscending = range.isReverse();
             } else {
                 this.step = step;
@@ -691,7 +707,7 @@
      */
     @SuppressWarnings("unchecked")
     private Comparable increment(Object value, Number step) {
-        return (Comparable) plus((Number) value, step);
+        return (Comparable) (increment != null ?
increment.apply((Number) value, step) : plus((Number) value, step));
     }

     /**
@@ -703,6 +719,6 @@
      */
     @SuppressWarnings("unchecked")
     private Comparable decrement(Object value, Number step) {
-        return (Comparable) minus((Number) value, step);
+        return (Comparable) (decrement != null ?
decrement.apply((Number) value, step) : minus((Number) value, step));
     }
 }

Re: Possible improvement to NumberRange

Posted by MG <mg...@arscreat.com>.
Hi guys,

some thoughts on Number sub-interfaces in general:

 1. I would have loved to have specialized Number sub-interfaces like
    FloatNumber and IntNumber for some time.
     1.  From what Paul suggests, RationalNumber would be another
        interesting interface to add here.
     2. Evidently an IntNumber should be a RationalNumber, while a
        RationalNumber should be a FloatNumber.
 2. Oftentimes I use Number when actually I know what I will have is
    e.g. an integer value ID, but alas there is no more specific
    interface that expresses that...
     1. ...while using e.g. BigInteger or BigDecimal classes can lead to
        e.g. "Long is not of type BigInteger" compile time errors.
 3. Tbh I have long wondered why no one on the Java side of things has
    ever seen the need to specialize Number at least into FloatNumber
    and IntNumber... (?)


More specific on the problem range of the Range problem:

 1. (Fraction.ONE..2).by(Fraction.ONE_QUARTER, Fraction::add,
    Fraction::subtract, Fraction::negate).toList()
     1. As is, that sound & looks very non-Groovy to me :-/
     2. If the user could register the shared part once somewhere and it
        would be picked up & used for all
        org.apache.commons.lang3.math.Fraction ranges, it would imho work.
 2. I also like Jochen's idea, framed in a Groovy way:
    (Fraction.ONE..2).step { it.add(Fraction.ONE_QUARTER) }
     1. (I am not sure why Jochen used (it ->
        it.add(Fraction.ONE_QUARTER)) here, I thought Groovy closure
        syntax would be mapped to Java lambda in simple cases like
        these... (?))
 3. A completely different approach might be to dynamically support some
    of the more common Number classes such as
    org.apache.commons.lang3.math.Fraction, by checking the canonical
    class name of the type passed to NumberRange before falling back to
    BigDecimal, and calling e.g. Fraction::add/subtract/negate
    dynamically on them as needed.
     1. If one caches the add/subtract/negate lambdas to use for an
        encountered Number class, this should be fast.
     2. This approach would be based on the assumption that there is a
        limited number of concrete Number classes out there people
        actually use.
     3. It would be very Groovy, since "the right thing" would happen
        automatically :-)
     4. Just to make Paul's day, I will suggest introducing emitting a
        Groovy compile time warning if a NumberRange type would be
        treated by the BigDecimal fallback, with the suggestion to
        contact the Groovy mailing list to request that support for the
        type be added to Groovy NumberRange >;-)
         1. (Or we could also support Jochen's idea from above, and
            suggest the user use that to avoid the BigDecimal fallback
            in a warning ;-) )

Cheers,
mg


On 10/01/2024 14:57, Paul King wrote:
> On Wed, Jan 10, 2024 at 8:41 PM Jochen Theodorou<bl...@gmx.org>  wrote:
>> On 10.01.24 05:53, Paul King wrote:
>>> Hi folks,
>>>
>>> The NumberRange abstraction tries very hard to allow any Number
>>> numeric type to be used but since the Number interface doesn't convey
>>> much behavior, there are a few places where it defaults to using
>>> Groovy's NumberMath plumbing which, to cut a long story short, falls
>>> back to using BigDecimal for any numeric calculations which aren't
>>> using the common known simpler types.
>> But wouldn't that mean we are lacking a range type for this?
> We could have yet another range type but NumberRange is currently
> coded to work for any Number subclass except in just a few places. And
> Fraction, being a Number subclass is what I was interested in here.
>
>>> A consequence of this is that currently if you created a range using
>>> e.g. the Apache Commons Fraction class (which does extend Number), and
>>> used a Fraction stepSize, the values in the range would be one
>>> Fraction (for the first element) and then subsequent elements would be
>>> BigDecimals.
>>>
>>> @Grab('org.apache.commons:commons-lang3:3.14.0')
>>> import org.apache.commons.lang3.math.Fraction
>>> def r = (Fraction.ONE..2).by(Fraction.ONE_QUARTER)
>>> println r.toList() // => [1/1, 1.25, 1.50, 1.75, 2.00]
>>>
>>> This isn't incorrect in one sense but is somewhat surprising. Given
>>> that the Number interface doesn't have operators, providing a smarter
>>> detection of the number system to use becomes somewhat tricky. One
>>> thing we could do is provide some interface that providers could use
>>> and we could have a "Fraction" math implementation that satisfied that
>>> interface.
>> But we have this convention already, which is substract, add and negate
>> as dynamic methods being called. It is not a formal interface, yes, but
>> so is also not for previous and next for the ObjectRange.
> We have multiply(-1), plus() and minus() but we don't call through to
> those methods directly but rather go through the NumberMath plumbing
> which ends up calling those methods for the Java Number classes but
> defaults back to BigDecimal for any unknown Number subclasses.
>
>>> Alternatively, we could supply some [Bi]Functions that
>>> offered the supplied behavior that the StepIterator needs when
>>> calculating subsequent elements in the range. With this second
>>> approach, we could do something like:
>>>
>>> @Grab('org.apache.commons:commons-lang3:3.14.0')
>>> import org.apache.commons.lang3.math.Fraction
>>> (Fraction.ONE..2).by(Fraction.ONE_QUARTER,
>>>       Fraction::add, Fraction::subtract, Fraction::negate).toList()
>>>
>>> Which gives a list of all Fraction instances: [1/1, 5/4, 3/2, 7/4, 2/1]
>>>
>>> Is this something we should support? Does anyone have ideas on the
>>> best implementation?
>> the key point about the Number interface is actually converting to one
>> of the Java numbers. Of course I understand what you want to achieve,
>> but is that really a NumberRange in the end?
>>
>> If I extend and abstract the idea above, don't I end up with something
>> where I define a step-function:
>>
>> (Fraction.ONE..2).by(it -> it.add(Fraction.ONE_QUARTER)
> Perhaps a step function is the way to go. The current implementation
> has some assumptions about counting up or down based on the sign of
> the step - that functionality might have to be abandoned if a step
> function was used instead of a step size.
>
>> and would that not kind of abstract ObjectRange as well:
>>
>> ('a'...'z').by(Character::next)
>>
>> I like here especially the simplicity, that you really only require
>> Comparable for this to work.
>>
>> Then again what does Number.next() do? It is calling plus(1). If we go
>> to the essence of it we actually have only one real range, which is the
>> ObjectRange and all other ranges are optimizations.
>>
>> In fact if you look at it strictly we are even breaking the contract by
>> not doing next in there, but plus(1). If you would replace next() on
>> Integer our IntRange will ignore that.
>>
>> Anyway, since they are optimization for specific cases maybe NumberRange
>> should not try to solve this, but a new range?
> We could define a new range type. What I am after is something which
> does everything the current NumberRange does but doesn't hard-code
> BigDecimal arithmetic for non-Java Number subclasses. I'll think a bit
> more about it.
>
>> bye Jochen

Re: Possible improvement to NumberRange

Posted by Paul King <pa...@asert.com.au>.
On Wed, Jan 10, 2024 at 8:41 PM Jochen Theodorou <bl...@gmx.org> wrote:
>
> On 10.01.24 05:53, Paul King wrote:
> > Hi folks,
> >
> > The NumberRange abstraction tries very hard to allow any Number
> > numeric type to be used but since the Number interface doesn't convey
> > much behavior, there are a few places where it defaults to using
> > Groovy's NumberMath plumbing which, to cut a long story short, falls
> > back to using BigDecimal for any numeric calculations which aren't
> > using the common known simpler types.
>
> But wouldn't that mean we are lacking a range type for this?

We could have yet another range type but NumberRange is currently
coded to work for any Number subclass except in just a few places. And
Fraction, being a Number subclass is what I was interested in here.

> > A consequence of this is that currently if you created a range using
> > e.g. the Apache Commons Fraction class (which does extend Number), and
> > used a Fraction stepSize, the values in the range would be one
> > Fraction (for the first element) and then subsequent elements would be
> > BigDecimals.
> >
> > @Grab('org.apache.commons:commons-lang3:3.14.0')
> > import org.apache.commons.lang3.math.Fraction
> > def r = (Fraction.ONE..2).by(Fraction.ONE_QUARTER)
> > println r.toList() // => [1/1, 1.25, 1.50, 1.75, 2.00]
> >
> > This isn't incorrect in one sense but is somewhat surprising. Given
> > that the Number interface doesn't have operators, providing a smarter
> > detection of the number system to use becomes somewhat tricky. One
> > thing we could do is provide some interface that providers could use
> > and we could have a "Fraction" math implementation that satisfied that
> > interface.
>
> But we have this convention already, which is substract, add and negate
> as dynamic methods being called. It is not a formal interface, yes, but
> so is also not for previous and next for the ObjectRange.

We have multiply(-1), plus() and minus() but we don't call through to
those methods directly but rather go through the NumberMath plumbing
which ends up calling those methods for the Java Number classes but
defaults back to BigDecimal for any unknown Number subclasses.

> > Alternatively, we could supply some [Bi]Functions that
> > offered the supplied behavior that the StepIterator needs when
> > calculating subsequent elements in the range. With this second
> > approach, we could do something like:
> >
> > @Grab('org.apache.commons:commons-lang3:3.14.0')
> > import org.apache.commons.lang3.math.Fraction
> > (Fraction.ONE..2).by(Fraction.ONE_QUARTER,
> >      Fraction::add, Fraction::subtract, Fraction::negate).toList()
> >
> > Which gives a list of all Fraction instances: [1/1, 5/4, 3/2, 7/4, 2/1]
> >
> > Is this something we should support? Does anyone have ideas on the
> > best implementation?
>
> the key point about the Number interface is actually converting to one
> of the Java numbers. Of course I understand what you want to achieve,
> but is that really a NumberRange in the end?
>
> If I extend and abstract the idea above, don't I end up with something
> where I define a step-function:
>
> (Fraction.ONE..2).by(it -> it.add(Fraction.ONE_QUARTER)

Perhaps a step function is the way to go. The current implementation
has some assumptions about counting up or down based on the sign of
the step - that functionality might have to be abandoned if a step
function was used instead of a step size.

> and would that not kind of abstract ObjectRange as well:
>
> ('a'...'z').by(Character::next)
>
> I like here especially the simplicity, that you really only require
> Comparable for this to work.
>
> Then again what does Number.next() do? It is calling plus(1). If we go
> to the essence of it we actually have only one real range, which is the
> ObjectRange and all other ranges are optimizations.
>
> In fact if you look at it strictly we are even breaking the contract by
> not doing next in there, but plus(1). If you would replace next() on
> Integer our IntRange will ignore that.
>
> Anyway, since they are optimization for specific cases maybe NumberRange
> should not try to solve this, but a new range?

We could define a new range type. What I am after is something which
does everything the current NumberRange does but doesn't hard-code
BigDecimal arithmetic for non-Java Number subclasses. I'll think a bit
more about it.

> bye Jochen

Re: Possible improvement to NumberRange

Posted by Jochen Theodorou <bl...@gmx.org>.
On 10.01.24 05:53, Paul King wrote:
> Hi folks,
>
> The NumberRange abstraction tries very hard to allow any Number
> numeric type to be used but since the Number interface doesn't convey
> much behavior, there are a few places where it defaults to using
> Groovy's NumberMath plumbing which, to cut a long story short, falls
> back to using BigDecimal for any numeric calculations which aren't
> using the common known simpler types.

But wouldn't that mean we are lacking a range type for this?

> A consequence of this is that currently if you created a range using
> e.g. the Apache Commons Fraction class (which does extend Number), and
> used a Fraction stepSize, the values in the range would be one
> Fraction (for the first element) and then subsequent elements would be
> BigDecimals.
>
> @Grab('org.apache.commons:commons-lang3:3.14.0')
> import org.apache.commons.lang3.math.Fraction
> def r = (Fraction.ONE..2).by(Fraction.ONE_QUARTER)
> println r.toList() // => [1/1, 1.25, 1.50, 1.75, 2.00]
>
> This isn't incorrect in one sense but is somewhat surprising. Given
> that the Number interface doesn't have operators, providing a smarter
> detection of the number system to use becomes somewhat tricky. One
> thing we could do is provide some interface that providers could use
> and we could have a "Fraction" math implementation that satisfied that
> interface.

But we have this convention already, which is substract, add and negate
as dynamic methods being called. It is not a formal interface, yes, but
so is also not for previous and next for the ObjectRange.

> Alternatively, we could supply some [Bi]Functions that
> offered the supplied behavior that the StepIterator needs when
> calculating subsequent elements in the range. With this second
> approach, we could do something like:
>
> @Grab('org.apache.commons:commons-lang3:3.14.0')
> import org.apache.commons.lang3.math.Fraction
> (Fraction.ONE..2).by(Fraction.ONE_QUARTER,
>      Fraction::add, Fraction::subtract, Fraction::negate).toList()
>
> Which gives a list of all Fraction instances: [1/1, 5/4, 3/2, 7/4, 2/1]
>
> Is this something we should support? Does anyone have ideas on the
> best implementation?

the key point about the Number interface is actually converting to one
of the Java numbers. Of course I understand what you want to achieve,
but is that really a NumberRange in the end?

If I extend and abstract the idea above, don't I end up with something
where I define a step-function:

(Fraction.ONE..2).by(it -> it.add(Fraction.ONE_QUARTER)

and would that not kind of abstract ObjectRange as well:

('a'...'z').by(Character::next)

I like here especially the simplicity, that you really only require
Comparable for this to work.

Then again what does Number.next() do? It is calling plus(1). If we go
to the essence of it we actually have only one real range, which is the
ObjectRange and all other ranges are optimizations.

In fact if you look at it strictly we are even breaking the contract by
not doing next in there, but plus(1). If you would replace next() on
Integer our IntRange will ignore that.

Anyway, since they are optimization for specific cases maybe NumberRange
should not try to solve this, but a new range?


bye Jochen