BigDecimal(0.0) is .equals to any other BigDecimal(double) with the same scale

[dankurka]

Originally reported on Google Code with ID 8646

Found in GWT Release (e.g. 2.4.0, 2.5.1, trunk):
2.6.0, trunk, but appears to be much older


Encountered on OS / Browser (e.g. WinXP, IE9, FF10):
FF and Chrome at least, prod/sdm only.


Detailed description (please be as specific as possible):
The BigDecimal supersource code can under certain cases report two numbers as being
.equals() to each other, even when they are clearly not. This only occurs (so far)
with the constructors that accept a double argument. The root issue seems to be that
when these values are close enough, their scale value is equal, but new BigDecimal(0.0)
has bitLength=0 (less than SMALL_VALUE_BITS) and both objects have the same smallValue.


According to BigDecimal.isZero, bitLength == 0 has some significance for zero valued
BigDecimal objects, so changing that wouldn't be good enough. Since new BigDecimal((int)0)
results in bitLength=0 but scale=0 instead of scale=19, it appears that modifying BigDecimal.toPrecision
could be the root issue instead, or that BigDecimal.initFrom should special case numbers
with all zeroes.


Shortest code snippet which demonstrates issue (please indicate where
actual result differs from expected result):

    Window.alert("0   == 1  ? " + new BigDecimal(0).equals(new BigDecimal(1)));
    Window.alert("0   == 1.0? " + new BigDecimal(0).equals(new BigDecimal(1.0)));
    Window.alert("0.0 == 1  ? " + new BigDecimal(0.0).equals(new BigDecimal(1)));
    Window.alert("0.0 == 1.0? " + new BigDecimal(0.0).equals(new BigDecimal(1.0)));//whoops


Workaround if you have one:
If you must wrap a BigDecimal around a double, first toString() the double:

    new BigDecimal("" + doubleValue)

It also appears possible to check for a doubleValue of 0 and use a different BigDecimal
constructor:

    new BigDecimal(0.0, 0.0)

As I don't fully understand the inner workings of BigDecimal, I can't promise that
these are fully comprehensive solutions, but they do get past this specific case.

Reported by niloc132 on 2014-03-28 18:06:09

[dankurka]

There is another case where this can happen, 0.0 is either not a special case after
all, or my guess about isZero was just a side effect of the bug, and not the root cause:

    Window.alert("1234 == 1234.0? " + new BigDecimal(1234).equals(new BigDecimal(1234d));//whoops

Reported by niloc132 on 2014-10-03 17:54:11

[bugear]

These are IMHO two different issues: the first is incorrect handling of smallValue and intVal representations in equals, while the issue in second post is caused by the fact, that behavior of GWT's double constructor differs from behavior of Java's.

Proposed fix of equals (based on compareTo implementation):

  /**
   * Returns {@code true} if {@code x} is a {@code BigDecimal} instance and if
   * this instance is equal to this big decimal. Two big decimals are equal if
   * their unscaled value and their scale is equal. For example, 1.0
   * (10*10^(-1)) is not equal to 1.00 (100*10^(-2)). Similarly, zero instances
   * are not equal if their scale differs.
   * 
   * @param x object to be compared with {@code this}.
   * @return true if {@code x} is a {@code BigDecimal} and {@code this == x}.
   */
  @Override
  public boolean equals(Object x) {
    if (this == x) {
      return true;
    }
    if (x instanceof BigDecimal) {
      BigDecimal x1 = (BigDecimal) x;

      if (this.scale == x1.scale && this.bitLength < SMALL_VALUE_BITS
          && x1.bitLength < SMALL_VALUE_BITS) {
        return (smallValue == x1.smallValue);
      }

      double diffScale = this.scale - x1.scale;
      double diffPrecision = this.approxPrecision() - x1.approxPrecision();
      if (this.scale != x1.scale || (diffPrecision > diffScale + 1) || (diffPrecision < diffScale - 1)) {
        return false;
      } else {
        BigInteger thisUnscaled = this.getUnscaledValue();
        BigInteger x1Unscaled = x1.getUnscaledValue();
        // If any of both precision is bigger, append zeros to the shorter one
        if (diffScale < 0) {
          thisUnscaled = thisUnscaled.multiply(Multiplication.powerOf10(-diffScale));
        } else if (diffScale > 0) {
          x1Unscaled = x1Unscaled.multiply(Multiplication.powerOf10(diffScale));
        }
        return thisUnscaled.equals(x1Unscaled);
      }
    }
    return false;
  }

[tbroyer]

Would you mind contributing the proposed fix to Gerrit?
(along with tests added to user/test/com/google/gwt/emultest/java/math/BigDecimalCompareTest.java)
http://www.gwtproject.org/makinggwtbetter.html#contributingcode

[bugear]

Commited as change 14877.

[tbroyer]

https://gwt-review.googlesource.com/14877

[niloc132]

(Merged about three weeks after it discussed, closing now since it was missed previously)