svn commit: r1336958 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math3/util/Pair.java test/java/org/apache/commons/math3/util/PairTest.java

[er...@apache.org]

Author: erans
Date: Thu May 10 23:36:07 2012
New Revision: 1336958

URL: http://svn.apache.org/viewvc?rev=1336958&view=rev
Log:
MATH-786
Reverting most of the changes performed in revision 1336458, because the
caching of the hash code could be the source of inconsistent behaviour,
and we are lacking proof that the performance gain is worth it.
The implementation (from r1336458) of the "hashCode" method is retained.

Modified:
    commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/Pair.java
    commons/proper/math/trunk/src/test/java/org/apache/commons/math3/util/PairTest.java

Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/Pair.java
URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/Pair.java?rev=1336958&r1=1336957&r2=1336958&view=diff
==============================================================================
--- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/Pair.java (original)
+++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/Pair.java Thu May 10 23:36:07 2012
@@ -34,51 +34,17 @@ public class Pair<K, V> {
     private final K key;
     /** Value. */
     private final V value;
-    /** Whether the pair contents can be assumed to be immutable. */
-    private final boolean isImmutable;
-    /** Cached has code. */
-    private final int cachedHashCode;
 
     /**
      * Create an entry representing a mapping from the specified key to the
      * specified value.
-     * If the pair can be assumed to be immutable, the hash code will be
-     * cached.
      *
      * @param k Key.
      * @param v Value.
-     * @param assumeImmutable Whether the pair contents can be assumed to
-     * be immutable.
      */
-    public Pair(K k, V v, boolean assumeImmutable) {
+    public Pair(K k, V v) {
         key = k;
         value = v;
-        isImmutable = assumeImmutable;
-        cachedHashCode = computeHashCode();
-    }
-
-    /**
-     * Create an entry representing a mapping from the specified key to the
-     * specified value.
-     *
-     * @param k Key.
-     * @param v Value.
-     */
-    public Pair(K k, V v) {
-        this(k, v, false);
-    }
-
-    /**
-     * Create an entry representing the same mapping as the specified entry.
-     * If the pair can be assumed to be immutable, the hash code will be
-     * cached.
-     *
-     * @param entry Entry to copy.
-     * @param assumeImmutable Whether the pair contents can be assumed to
-     * be immutable.
-     */
-    public Pair(Pair<? extends K, ? extends V> entry, boolean assumeImmutable) {
-        this(entry.getKey(), entry.getValue(), assumeImmutable);
     }
 
     /**
@@ -87,7 +53,7 @@ public class Pair<K, V> {
      * @param entry Entry to copy.
      */
     public Pair(Pair<? extends K, ? extends V> entry) {
-        this(entry, false);
+        this(entry.getKey(), entry.getValue());
     }
 
     /**
@@ -125,11 +91,11 @@ public class Pair<K, V> {
         } else {
             Pair<?, ?> oP = (Pair<?, ?>) o;
             return (key == null ?
-                    oP.getKey() == null :
-                    key.equals(oP.getKey())) &&
+                    oP.key == null :
+                    key.equals(oP.key)) &&
                 (value == null ?
-                 oP.getValue() == null :
-                 value.equals(oP.getValue()));
+                 oP.value == null :
+                 value.equals(oP.value));
         }
     }
 
@@ -140,15 +106,6 @@ public class Pair<K, V> {
      */
     @Override
     public int hashCode() {
-        return isImmutable ? cachedHashCode : computeHashCode();
-    }
-
-    /**
-     * Compute a hash code.
-     *
-     * @return the hash code value.
-     */
-    private final int computeHashCode() {
         int result = key == null ? 0 : key.hashCode();
 
         final int h = value == null ? 0 : value.hashCode();

Modified: commons/proper/math/trunk/src/test/java/org/apache/commons/math3/util/PairTest.java
URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/test/java/org/apache/commons/math3/util/PairTest.java?rev=1336958&r1=1336957&r2=1336958&view=diff
==============================================================================
--- commons/proper/math/trunk/src/test/java/org/apache/commons/math3/util/PairTest.java (original)
+++ commons/proper/math/trunk/src/test/java/org/apache/commons/math3/util/PairTest.java Thu May 10 23:36:07 2012
@@ -60,24 +60,6 @@ public class PairTest {
         // Different contents, different hash codes.
         m2.set(2);
         Assert.assertFalse(p1.hashCode() == p2.hashCode());
-
-        // Test cache.
-
-        final MyInteger m3 = new MyInteger(1);
-        final Pair<MyInteger, MyInteger> p3 = new Pair<MyInteger, MyInteger>(m3, m3, true);
-        final int hC3 = p3.hashCode();
-        // Contents change will not affect the hash code because it is cached.
-        m3.set(3);
-        Assert.assertTrue(hC3 == p3.hashCode());
-
-        final Pair<MyInteger, MyInteger> p4 = new Pair<MyInteger, MyInteger>(p3, false);
-        // p3 and p4 do not have the same hash code because p4 was contructed after m3
-        // was changed.
-        Assert.assertFalse(p4.hashCode() == p3.hashCode());
-        final int hC4 = p4.hashCode();
-        // Contents change will affect the hash code because it is not cached.
-        m3.set(4);
-        Assert.assertFalse(hC4 == p4.hashCode());
     }
 
     /**