[
https://issues.apache.org/jira/browse/MATH-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18063634#comment-18063634
]
Alex Herbert commented on MATH-1686:
------------------------------------
I don't think that this matters. The OP stated the issue was picked up by an
automated tool which is targeted at checking equals and hashCode for adding
objects to collections.
With the current implementation two Dfp cannot be equal if they have different
radix digits (the number of internal digits used in the representation). This
can lead to two numbers with different internal representations being not equal.
{code:java}
DfpField f1 = new DfpField(10);
DfpField f2 = new DfpField(100);
f1.newDfp(1).equals(f2.newDfp(1)); // false
{code}
So it is not a strict numerical equality. This is in contrast for example to
BigInteger which only has 1 internal representation.
I think it is fine to document this as failing the equals contract. If we fix
the NaN issue then NaNs can be equal but two numbers which are *numerically
equal* could fail to be measured as equal (unless that is fixed too).
Documenting it as an exact equality of the internal binary representation of a
*finite* number would match the implementation.
No end user should be adding Dfp to collections for counting (with a set) or as
a key to a map. The main value of Dfp is an alternative to using BigDecimal
when you want elementary functions to high precision such as sin, cos, exp, etc.
> `Dfp.equals()` returns `false` for `NaN.equals(NaN)`, violating
> `Object.equals()` reflexivity
> ---------------------------------------------------------------------------------------------
>
> Key: MATH-1686
> URL: https://issues.apache.org/jira/browse/MATH-1686
> Project: Commons Math
> Issue Type: Bug
> Reporter: Shan Jiang
> Priority: Major
>
> # Summary
> `Dfp.equals()` returns `false` when both operands are NaN, violating the
> reflexivity requirement
> of `Object.equals()` (JDK 21 Javadoc):
> {noformat}
> "The `equals` method implements an equivalence relation on non-null object
> references: It is
> *reflexive*: for any non-null reference value `x`, `x.equals(x)` should
> return `true`."
> {noformat}
> # Root cause
> In [`Dfp.java` lines
> 895-907|https://github.com/apache/commons-math/blob/master/commons-math-legacy-core/src/main/java/org/apache/commons/math4/legacy/core/dfp/Dfp.java#L895-L907]:
> {code}
> @Override
> public boolean equals(final Object other) {
> if (other instanceof Dfp) {
> finalDfpx = (Dfp) other;
> if (isNaN() || x.isNaN() || field.getRadixDigits() !=
> x.field.getRadixDigits()) {
> returnfalse; // <-- NaN.equals(NaN) returns false
> }
> returncompare(this, x) == 0;
> }
> returnfalse;
> }
> {code}
> # Reproducer
> {code}
> Dfp nan = new DfpField(20).newDfp().newInstance((byte) 1, Dfp.QNAN);
> System.out.println(nan.equals(nan)); // false (should be true)
> {code}
> # Comparison with JDK
> The JDK's `Double.equals()` and `Float.equals()` deliberately return `true`
> for NaN:
> {noformat}
> `Double.equals()`: "If `d1` represents `+0.0` while `d2` represents `-0.0`,
> or vice versa,
> the `equal` test has the value `false`, even though `+0.0==-0.0` has the
> value `true`. [...]
> If `d1` and `d2` both represent `Double.NaN`, then the `equals` method
> returns `true`, even
> though `Double.NaN==Double.NaN` has the value `false`."
> {noformat}
> The JDK explicitly chose to break IEEE 754 NaN semantics in `equals()` to
> preserve the
> `Object.equals()` contract. `Dfp` should follow the same convention.
> # Impact
> Any `Dfp` NaN value placed in a `HashSet`, `HashMap`, or compared with
> `equals()` will behave
> incorrectly:
> ** `set.add(nan); set.contains(nan)` → may return `false`
> ** `map.put(nan, value); map.get(nan)` → may return `null`
> # How this was found
> Detected by an automated JDK conformance oracle
> (`ObjectOracles.checkEqualsReflexive`).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
