Avoid calling BigFraction#reduce() in BigFraction#equals() method

Hipparchus development
1

Hi

Why are there calls to BigFraction#reduce() in the BigFraction#equals() method?
Shouldn’t the BigFraction’s be reduced already in the constructors?

Even worse if reduce() would really detect a gcd, the gcd calculation would run a second time in the BigFraction() constructor:

2

Good catch.
I think you are right.
Did you check if the unit test run properly when removing this probably spurious reduction?

3

No I didn’t checked JUnit cases. I’ve found it in performance profiling.

4

I removed the reduce() method in equals and tested again with JUnit test cases of my project and found no problem:

public boolean equals(final Object other) {
    boolean ret = false;

    if (this == other) {
        ret = true;
    } else if (other instanceof BigFraction) {
        BigFraction rhs = ((BigFraction) other);  
        BigFraction thisOne = this; 
        ret = thisOne.numerator.equals(rhs.numerator) && thisOne.denominator.equals(rhs.denominator);
    }

    return ret;
}
5

So this is really a performance bug.

could you open the issue?

By the way, I am in the process of releasing a new minor version (1.8), not 2.0 yet unfortunately, but thinking about it.

6

Done: https://github.com/Hipparchus-Math/hipparchus/issues/105

7

Just looking at commons-number:

Why do they use the abs() method?