Skip to content

Commit 663885e

Browse files
committed
Improve node number equality
Attempts to use the most optimal comparisons of number nodes, using .equals if both number values are of the same type, then the string representation of each value, finally followed by comparing the BigDecimal representation of each value using the string cache. If a BigDecimal is needed, it is cached on the number node for future checks. Closes #1378
1 parent e10e6be commit 663885e

File tree

2 files changed

+88
-1
lines changed

2 files changed

+88
-1
lines changed

smithy-model/src/main/java/software/amazon/smithy/model/node/NumberNode.java

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,16 @@ public final class NumberNode extends Node {
3333

3434
private final Number value;
3535
private final String stringCache;
36+
private volatile BigDecimal equality;
3637

3738
public NumberNode(Number value, SourceLocation sourceLocation) {
3839
super(sourceLocation);
3940
this.value = Objects.requireNonNull(value);
4041
stringCache = value.toString();
42+
43+
if (value instanceof BigDecimal) {
44+
equality = (BigDecimal) value;
45+
}
4146
}
4247

4348
/**
@@ -129,7 +134,47 @@ public boolean isZero() {
129134

130135
@Override
131136
public boolean equals(Object other) {
132-
return other instanceof NumberNode && stringCache.equals(((NumberNode) other).stringCache);
137+
if (!(other instanceof NumberNode)) {
138+
return false;
139+
} else if (other == this) {
140+
return true;
141+
} else {
142+
NumberNode otherNode = (NumberNode) other;
143+
144+
// This only works if both values are the same type.
145+
if (value.equals(otherNode.value)) {
146+
return true;
147+
}
148+
149+
// Attempt a cheap check based on the string cache.
150+
if (stringCache.equals(otherNode.stringCache)) {
151+
return true;
152+
}
153+
154+
// Convert both to BigDecimal and compare equality.
155+
return getEquality().equals(otherNode.getEquality());
156+
}
157+
}
158+
159+
private BigDecimal getEquality() {
160+
BigDecimal e = equality;
161+
162+
if (e == null) {
163+
if (value instanceof Integer) {
164+
e = BigDecimal.valueOf(value.intValue());
165+
} else if (value instanceof Short) {
166+
e = BigDecimal.valueOf(value.shortValue());
167+
} else if (value instanceof Byte) {
168+
e = BigDecimal.valueOf(value.byteValue());
169+
} else if (value instanceof Long) {
170+
e = BigDecimal.valueOf(value.longValue());
171+
} else {
172+
e = new BigDecimal(stringCache);
173+
}
174+
equality = e;
175+
}
176+
177+
return e;
133178
}
134179

135180
@Override

smithy-model/src/test/java/software/amazon/smithy/model/node/NumberNodeTest.java

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515

1616
package software.amazon.smithy.model.node;
1717

18+
import static org.hamcrest.CoreMatchers.equalTo;
1819
import static org.hamcrest.CoreMatchers.instanceOf;
20+
import static org.hamcrest.CoreMatchers.not;
1921
import static org.hamcrest.MatcherAssert.assertThat;
2022
import static org.junit.jupiter.api.Assertions.assertEquals;
2123
import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -241,4 +243,44 @@ public String toString() {
241243
assertEquals(v, result, "Expected " + k + " to be " + v);
242244
});
243245
}
246+
247+
@Test
248+
public void compareIntAndShort() {
249+
Node left = Node.from(1);
250+
Node right = Node.from((short) 1);
251+
252+
assertThat(left, equalTo(right));
253+
}
254+
255+
@Test
256+
public void compareShortAndFloat() {
257+
Node left = Node.from((float) 1.0);
258+
Node right = Node.from((short) 1);
259+
260+
assertThat(left, not(equalTo(right)));
261+
}
262+
263+
@Test
264+
public void compareByteAndFloat() {
265+
Node left = Node.from((float) 1.0);
266+
Node right = Node.from((byte) 1);
267+
268+
assertThat(left, not(equalTo(right)));
269+
}
270+
271+
@Test
272+
public void compareDoubleAndBigDecimal() {
273+
Node left = Node.from(new BigDecimal("1.0e+10"));
274+
Node right = Node.from(1.0e+10);
275+
276+
assertThat(left, equalTo(right));
277+
}
278+
279+
@Test
280+
public void compareDoubleAndFloat() {
281+
Node left = Node.from((float) 1.0e+10);
282+
Node right = Node.from((double) 1.0e+10);
283+
284+
assertThat(left, equalTo(right));
285+
}
244286
}

0 commit comments

Comments
 (0)