/issues/146 Fixed hashCode/equals.

JavaMoney · Jul 30, 2017 · e83cb97 · e83cb97
1 parent cc7860f
commit e83cb97
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 24 deletions.
diff --git a/...a-convert/moneta-convert-base/src/main/java/org/javamoney/moneta/DefaultExchangeRate.java b/...a-convert/moneta-convert-base/src/main/java/org/javamoney/moneta/DefaultExchangeRate.java
@@ -16,6 +16,7 @@
 package org.javamoney.moneta;
 
 import java.io.Serializable;
+import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
@@ -259,7 +260,23 @@ public String toString() {
      */
     @Override
     public int hashCode() {
-        return Objects.hash(base, conversionContext, factor, term, chain);
+        // Numerically equal factors should produce the same hash code, so hash a normalized
+        // version of the factor rather than the NumberValue object.
+        BigDecimal normalizedFactor = factor.numberValue(BigDecimal.class).stripTrailingZeros();
+
+        // The exchange rate chain includes a reference to "this" if the caller doesn't explicitly
+        // set a chain in the builder, so we can't naively hash the chain or we'll get infinite
+        // recursion.
+        int chainHash = 0;
+        for (ExchangeRate chainedRate : chain) {
+            if (chainedRate == this) {
+                // Use a constant to represent the presence of this object in the chain.
+                chainHash = Objects.hash(chainHash, "this");
+            } else {
+                chainHash = Objects.hash(chainHash, chainedRate);
+            }
+        }
+        return Objects.hash(base, conversionContext, normalizedFactor, term, chainHash);
     }
 
     /*
@@ -272,13 +289,11 @@ public boolean equals(Object obj) {
         if (obj == this) {
             return true;
         }
-        if (obj instanceof DefaultExchangeRate) {
-            DefaultExchangeRate other = (DefaultExchangeRate) obj;
-            return Objects.equals(base, other.base) &&
-                    Objects.equals(conversionContext, other.conversionContext) &&
-                    Objects.equals(factor, other.factor) && Objects.equals(term, other.term);
-        }
-        return false;
+        DefaultExchangeRate other = (DefaultExchangeRate) obj;
+        return Objects.equals(base, other.base) &&
+                Objects.equals(conversionContext, other.conversionContext) &&
+                factor.compareTo(other.factor) == 0 &&
+                Objects.equals(term, other.term);
     }
 
     /**

diff --git a/...t/moneta-convert-base/src/main/java/org/javamoney/moneta/convert/DefaultExchangeRate.java b/...t/moneta-convert-base/src/main/java/org/javamoney/moneta/convert/DefaultExchangeRate.java
@@ -289,14 +289,11 @@ public boolean equals(Object obj) {
         if (obj == this) {
             return true;
         }
-        if (obj instanceof DefaultExchangeRate) {
-            DefaultExchangeRate other = (DefaultExchangeRate) obj;
-            return Objects.equals(base, other.base) &&
-                    Objects.equals(conversionContext, other.conversionContext) &&
-                    factor.compareTo(other.factor) == 0 &&
-                    Objects.equals(term, other.term);
-        }
-        return false;
+        DefaultExchangeRate other = (DefaultExchangeRate) obj;
+        return Objects.equals(base, other.base) &&
+                Objects.equals(conversionContext, other.conversionContext) &&
+                factor.compareTo(other.factor) == 0 &&
+                Objects.equals(term, other.term);
     }
 
     /**

diff --git a/moneta-core/src/main/java/org/javamoney/moneta/DefaultExchangeRate.java b/moneta-core/src/main/java/org/javamoney/moneta/DefaultExchangeRate.java
@@ -16,6 +16,7 @@
 package org.javamoney.moneta;
 
 import java.io.Serializable;
+import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
@@ -259,7 +260,23 @@ public String toString() {
      */
     @Override
     public int hashCode() {
-        return Objects.hash(base, conversionContext, factor, term, chain);
+        // Numerically equal factors should produce the same hash code, so hash a normalized
+        // version of the factor rather than the NumberValue object.
+        BigDecimal normalizedFactor = factor.numberValue(BigDecimal.class).stripTrailingZeros();
+
+        // The exchange rate chain includes a reference to "this" if the caller doesn't explicitly
+        // set a chain in the builder, so we can't naively hash the chain or we'll get infinite
+        // recursion.
+        int chainHash = 0;
+        for (ExchangeRate chainedRate : chain) {
+            if (chainedRate == this) {
+                // Use a constant to represent the presence of this object in the chain.
+                chainHash = Objects.hash(chainHash, "this");
+            } else {
+                chainHash = Objects.hash(chainHash, chainedRate);
+            }
+        }
+        return Objects.hash(base, conversionContext, normalizedFactor, term, chainHash);
     }
 
     /*
@@ -272,13 +289,11 @@ public boolean equals(Object obj) {
         if (obj == this) {
             return true;
         }
-        if (obj instanceof DefaultExchangeRate) {
-            DefaultExchangeRate other = (DefaultExchangeRate) obj;
-            return Objects.equals(base, other.base) &&
-                    Objects.equals(conversionContext, other.conversionContext) &&
-                    Objects.equals(factor, other.factor) && Objects.equals(term, other.term);
-        }
-        return false;
+        DefaultExchangeRate other = (DefaultExchangeRate) obj;
+        return Objects.equals(base, other.base) &&
+                Objects.equals(conversionContext, other.conversionContext) &&
+                factor.compareTo(other.factor) == 0 &&
+                Objects.equals(term, other.term);
     }
 
     /**