The code to compare two numbers in json object/array does not correct…

…ly compare BigInteger/BigDecimal values. Fix this issue and also simplify / refactor this code to be more compact and reused between object/array
eclipse-vertx · Jun 17, 2024 · 392a001 · 392a001
1 parent e56e961
commit 392a001
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 97 deletions.
diff --git a/src/main/java/io/vertx/core/json/JsonArray.java b/src/main/java/io/vertx/core/json/JsonArray.java
@@ -20,6 +20,7 @@
 import java.util.function.Function;
 import java.util.stream.Stream;
 
+import static io.vertx.core.json.JsonObject.compareObjects;
 import static io.vertx.core.json.impl.JsonUtil.*;
 import static java.time.format.DateTimeFormatter.ISO_INSTANT;
 
@@ -644,42 +645,7 @@ public boolean equals(Object o) {
     for (int i = 0; i < this.size(); i++) {
       Object thisValue = this.getValue(i);
       Object otherValue = other.getValue(i);
-      // identity check
-      if (thisValue == otherValue) {
-        continue;
-      }
-      // special case for numbers
-      if (thisValue instanceof Number && otherValue instanceof Number && thisValue.getClass() != otherValue.getClass()) {
-        Number n1 = (Number) thisValue;
-        Number n2 = (Number) otherValue;
-        // floating point values
-        if (thisValue instanceof Float || thisValue instanceof Double || otherValue instanceof Float || otherValue instanceof Double) {
-          // compare as floating point double
-          if (n1.doubleValue() == n2.doubleValue()) {
-            // same value check the next entry
-            continue;
-          }
-        }
-        if (thisValue instanceof Integer || thisValue instanceof Long || otherValue instanceof Integer || otherValue instanceof Long) {
-          // compare as integer long
-          if (n1.longValue() == n2.longValue()) {
-            // same value check the next entry
-            continue;
-          }
-        }
-      }
-      // special case for char sequences
-      if (thisValue instanceof CharSequence && otherValue instanceof CharSequence && thisValue.getClass() != otherValue.getClass()) {
-        CharSequence s1 = (CharSequence) thisValue;
-        CharSequence s2 = (CharSequence) otherValue;
-
-        if (Objects.equals(s1.toString(), s2.toString())) {
-          // same value check the next entry
-          continue;
-        }
-      }
-      // fallback to standard object equals checks
-      if (!Objects.equals(thisValue, otherValue)) {
+      if (thisValue != otherValue && !compareObjects(thisValue, otherValue)) {
         return false;
       }
     }

diff --git a/src/main/java/io/vertx/core/json/JsonObject.java b/src/main/java/io/vertx/core/json/JsonObject.java
@@ -1158,6 +1158,7 @@ public String toString() {
     return encode();
   }
 
+
   @Override
   public boolean equals(Object o) {
     // null check
@@ -1182,72 +1183,58 @@ public boolean equals(Object o) {
 
       Object thisValue = this.getValue(key);
       Object otherValue = other.getValue(key);
-      // identity check
-      if (thisValue == otherValue) {
-        continue;
+      if (thisValue != otherValue && !compareObjects(thisValue, otherValue)) {
+        return false;
       }
-      // special case for numbers
-
-      if (thisValue instanceof Number && otherValue instanceof Number) {
-        if (thisValue.getClass() == otherValue.getClass()) {
-          if (thisValue.equals(otherValue)) {
-            continue;
-          }
-        } else {
-          // meaning that the numbers are different types
-          Number n1 = (Number) thisValue;
-          Number n2 = (Number) otherValue;
-          if ((thisValue instanceof Float || thisValue instanceof Double) &&
-            (otherValue instanceof Float || otherValue instanceof Double)) {
-            // compare as floating point double
-            if (n1.doubleValue() == n2.doubleValue()) {
-              // same value, check the next entry
-              continue;
-            }
-          }
-
-          if ((thisValue instanceof Integer || thisValue instanceof Long) &&
-            (otherValue instanceof Integer || otherValue instanceof Long)) {
-            // compare as integer long
-            if (n1.longValue() == n2.longValue()) {
-              // same value, check the next entry
-              continue;
-            }
-          }
-
+    }
+    // all checks passed
+    return true;
+  }
 
-          // if its either integer or long and the other is float or double or vice versa,
-          // compare as floating point double
-          if ((thisValue instanceof Integer || thisValue instanceof Long) &&
-            (otherValue instanceof Float || otherValue instanceof Double) ||
-            (thisValue instanceof Float || thisValue instanceof Double) &&
-              (otherValue instanceof Integer || otherValue instanceof Long)) {
-            // compare as floating point double
-            if (n1.doubleValue() == n2.doubleValue()) {
-              // same value, check the next entry
-              continue;
-            }
-          }
-        }
+  static boolean compareObjects(Object o1, Object o2) {
+    if (o1 instanceof Number && o2 instanceof Number) {
+      if (o1.getClass() == o2.getClass()) {
+        return o1.equals(o2);
+      } else {
+        // meaning that the numbers are different types
+        Number n1 = (Number) o1;
+        Number n2 = (Number) o2;
+        return compareNumbers(n1, n2);
       }
+    } else if (o1 instanceof CharSequence && o2 instanceof CharSequence && o1.getClass() != o2.getClass()) {
+      return Objects.equals(o1.toString(), o2.toString());
+    } else {
+      return Objects.equals(o1, o2);
+    }
+  }
 
-      // special case for char sequences
-      if (thisValue instanceof CharSequence && otherValue instanceof CharSequence && thisValue.getClass() != otherValue.getClass()) {
-        CharSequence s1 = (CharSequence) thisValue;
-        CharSequence s2 = (CharSequence) otherValue;
-
-        if (Objects.equals(s1.toString(), s2.toString())) {
-          // same value, check the next entry
-          continue;
-        }
-      }
-      // fallback to standard object equals checks
-      if (!Objects.equals(thisValue, otherValue)) {
-        return false;
+  private static boolean compareNumbers(Number n1, Number n2) {
+    if (isDecimalNumber(n1) && isDecimalNumber(n2)) {
+      // compare as floating point double
+      return n1.doubleValue() == n2.doubleValue();
+    } else if (isWholeNumber(n1) && isWholeNumber(n2)) {
+      // compare as integer long
+      return n1.longValue() == n2.longValue();
+    } else if (isWholeNumber(n1) && isDecimalNumber(n2) ||
+      isDecimalNumber(n1) && isWholeNumber(n2)) {
+      // if its either integer or long and the other is float or double or vice versa,
+      // compare as floating point double
+      return n1.doubleValue() == n2.doubleValue();
+    } else {
+      if (isWholeNumber(n1)) {
+        return n1.longValue() == n2.longValue();
+      } else  {
+        return n1.doubleValue() == n2.doubleValue();
       }
     }
-    // all checks passed
-    return true;
+  }
+
+  private static boolean isWholeNumber(Number thisValue) {
+    return thisValue instanceof Integer || thisValue instanceof Long;
+  }
+
+  private static boolean isDecimalNumber(Number thisValue) {
+    return thisValue instanceof Float || thisValue instanceof Double;
   }
 
   @Override

diff --git a/src/test/java/io/vertx/core/json/JsonObjectTest.java b/src/test/java/io/vertx/core/json/JsonObjectTest.java
@@ -1578,7 +1578,16 @@ public void testNumberEquality() {
     assertNumberEquals(2D, 2);
     assertNumberNotEquals(2.3D, 2);
     assertNumberNotEquals(2.3f, 2);
-
+    assertNumberEquals(2, new BigDecimal(2));
+    assertNumberEquals(new BigDecimal(2), 2);
+    assertNumberEquals(2D, new BigDecimal(2));
+    assertNumberEquals(new BigDecimal(2), 2D);
+    assertNumberEquals(2, BigInteger.valueOf(2));
+    assertNumberEquals(BigInteger.valueOf(2), 2);
+    assertNumberEquals(2D, BigInteger.valueOf(2));
+    assertNumberEquals(BigInteger.valueOf(2), 2D);
+    assertNumberEquals(BigInteger.valueOf(2), new BigDecimal(2));
+    assertNumberEquals(new BigDecimal(2), BigInteger.valueOf(2));
   }
 
   private void assertNumberEquals(Number value1, Number value2) {