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


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:

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

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

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;

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.

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

Just looking at commons-number:

Why do they use the abs() method?