From: Travis Keep <keep94@gmail.com>
Date: Tue, 17 Dec 2013 23:16:55 +0000 (+0000)
Subject: ICU-10268 Fix equals() / hashCode() according to effective JAVA in MeasureFormat... 
X-Git-Tag: milestone-59-0-1~2321
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e79533c9ad5389fe0b258cff025d26d8325cd970;p=icu

ICU-10268 Fix equals() / hashCode() according to effective JAVA in MeasureFormat. Take out synchronized blocks in CurrencyFormat as this class will remain thread unsafe.

X-SVN-Rev: 34786
---

diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/CurrencyFormat.java b/icu4j/main/classes/core/src/com/ibm/icu/text/CurrencyFormat.java
index 0af58a87ce7..39634847e55 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/text/CurrencyFormat.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/text/CurrencyFormat.java
@@ -65,11 +65,8 @@ class CurrencyFormat extends MeasureFormat {
         }
         CurrencyAmount currency = (CurrencyAmount) obj;
             
-        // Since we extend MeasureFormat, we have to maintain thread-safety.
-        synchronized (fmt) {
-            fmt.setCurrency(currency.getCurrency());
-            return fmt.format(currency.getNumber(), toAppendTo, pos);
-        }
+        fmt.setCurrency(currency.getCurrency());
+        return fmt.format(currency.getNumber(), toAppendTo, pos);
     }
 
     /**
@@ -78,9 +75,7 @@ class CurrencyFormat extends MeasureFormat {
      */
     @Override
     public CurrencyAmount parseObject(String source, ParsePosition pos) {
-        synchronized (fmt) {
-            return fmt.parseCurrency(source, pos);
-        }
+        return fmt.parseCurrency(source, pos);
     }
     
     // boilerplate code to make CurrencyFormat otherwise follow the contract of
@@ -144,20 +139,6 @@ class CurrencyFormat extends MeasureFormat {
     
     // End boilerplate.
     
-    /**
-     * @draft ICU 53
-     * @provisional
-     */
-    @Override
-    public int hashCode() {
-        return mf.hashCode() + 154321962;
-    }
-    
-    @Override
-    boolean equalsSameClass(MeasureFormat other) {
-        return mf.equals(((CurrencyFormat) other).mf);
-    }
-    
     // Serialization
     
     private Object writeReplace() throws ObjectStreamException {
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/MeasureFormat.java b/icu4j/main/classes/core/src/com/ibm/icu/text/MeasureFormat.java
index 38b6d2a5306..e8c3957fdf6 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/text/MeasureFormat.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/text/MeasureFormat.java
@@ -80,9 +80,10 @@ import com.ibm.icu.util.UResourceBundle;
  * This class does not do conversions from one unit to another. It simply formats
  * whatever units it is given
  * <p>
- * This class is immutable so long as no mutable subclass, namely TimeUnitFormat,
- * is used. Although this class has existing subclasses, this class does not
- * support new sub-classes.   
+ * This class is immutable and thread-safe so long as its subclasses TimeUnitFormat and
+ * CurrencyFormat are never used. Neither subclass is thread-safe and TimeUnitFormat is
+ * mutable. Although this class has existing subclasses, this class does not support new
+ * sub-classes.   
  *
  * @see com.ibm.icu.text.UFormat
  * @author Alan Liu
@@ -365,9 +366,8 @@ public class MeasureFormat extends UFormat {
     }
     
     /**
-     * For two MeasureFormat objects, a and b, to be equal, <code>a.getClass().equals(b.getClass())</code>
-     * <code>a.equalsSameClass(b)</code> must be true.
-     * 
+     * To MeasureFormats, a and b, are equal if and only if they have the same width,
+     * locale, and equal number formats.
      * @draft ICU 53
      * @provisional
      */
@@ -379,10 +379,11 @@ public class MeasureFormat extends UFormat {
         if (!(other instanceof MeasureFormat)) {
             return false;
         }
-        if (!getClass().equals(other.getClass())) {
-            return false;
-        }
-        return equalsSameClass((MeasureFormat) other);
+        MeasureFormat rhs = (MeasureFormat) other;
+        // A very slow but safe implementation.
+        return getWidth() == rhs.getWidth() 
+                && getLocale().equals(rhs.getLocale()) 
+                && getNumberFormat().equals(rhs.getNumberFormat());
     }
     
     /**
@@ -390,20 +391,12 @@ public class MeasureFormat extends UFormat {
      * @provisional
      */
     @Override
-    public int hashCode() {
-        return (numberFormat.hashCode() * 31 + getLocale().hashCode()) * 31 + length.hashCode();
-    }
-    
-    /**
-     * Returns true if this object is equal to other. The class of this and the class of other
-     * are guaranteed to be equal.
-     */
-    boolean equalsSameClass(MeasureFormat other) {
-        return objEquals(numberFormat,other.numberFormat)
-                && objEquals(getLocale(), other.getLocale()) && objEquals(length, other.length);        
+    public final int hashCode() {
+        // A very slow but safe implementation.
+        return (getLocale().hashCode() * 31 
+                + getNumberFormat().hashCode()) * 31 + getWidth().hashCode();
     }
     
-    
     /**
      * Get the format width this instance is using.
      * @draft ICU 53
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/TimeUnitFormat.java b/icu4j/main/classes/core/src/com/ibm/icu/text/TimeUnitFormat.java
index 2f95bf51fa2..60726ebe633 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/text/TimeUnitFormat.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/text/TimeUnitFormat.java
@@ -563,16 +563,6 @@ if ( searchPluralCount.equals("other") ) {
         return mf.getLocale();
     }
     
-    /**
-     * @draft ICU 53
-     * @provisional
-     */
-    @Override
-    public Object clone() {
-        TimeUnitFormat result = (TimeUnitFormat) super.clone();
-        result.format = (NumberFormat) format.clone();
-        return result;
-    }
     
     /**
      * @draft ICU 53
@@ -583,25 +573,17 @@ if ( searchPluralCount.equals("other") ) {
         return mf.getNumberFormat();
     }
     
-    // End boilerplate.
-    
-    // equals / hashcode
-    
     /**
      * @draft ICU 53
      * @provisional
      */
     @Override
-    public int hashCode() {
-        return mf.hashCode() + 911247101;
-    }
-    
-    @Override
-    boolean equalsSameClass(MeasureFormat other) {
-        return mf.equals(((TimeUnitFormat) other).mf);
+    public Object clone() {
+        TimeUnitFormat result = (TimeUnitFormat) super.clone();
+        result.format = (NumberFormat) format.clone();
+        return result;
     }
-    
-    // End equals / hashcode
+    // End boilerplate.
     
     // Serialization