Updated logic in put() to remove pair on duplicate value. Because of this, TestBidiMap can no longer extend TestMap since this seems to break the Map contract.

git-svn-id: https://svn.apache.org/repos/asf/jakarta/commons/proper/collections/trunk@131193 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/commons/collections/HashBidiMap.java

@@ -1,5 +1,5 @@
 /*
- * $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/java/org/apache/commons/collections/Attic/HashBidiMap.java,v 1.1 2003/09/23 20:29:34 matth Exp $
+ * $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/java/org/apache/commons/collections/Attic/HashBidiMap.java,v 1.2 2003/09/26 23:28:43 matth Exp $
  * ====================================================================
  *
  * The Apache Software License, Version 1.1
@@ -57,6 +57,7 @@
  */
 package org.apache.commons.collections;
 
+import java.io.Serializable;
 import java.util.AbstractMap;
 import java.util.AbstractSet;
 import java.util.HashMap;
@@ -68,14 +69,14 @@ import java.util.Set;
  * Default implementation of <code>BidiMap</code>.
  * 
  * @since Commons Collections 3.0
- * @version $Id: HashBidiMap.java,v 1.1 2003/09/23 20:29:34 matth Exp $
+ * @version $Id: HashBidiMap.java,v 1.2 2003/09/26 23:28:43 matth Exp $
  * 
  * @author Matthew Hawthorne
  */
-public class HashBidiMap extends AbstractMap implements BidiMap {
+public class HashBidiMap extends AbstractMap implements BidiMap, Serializable {
 
     /**
-     * Delegate map array.  The first map contains standards entries, and the 
+     * Delegate map array.  The first map contains standard entries, and the 
      * second contains inverses.
      */
     final Map[] maps = new Map[] { new HashMap(), new HashMap()};
@@ -114,31 +115,36 @@ public class HashBidiMap extends AbstractMap implements BidiMap {
     }
 
     public Object put(Object key, Object value) {
+        // Removes pair from standard map if a previous inverse entry exists
+        final Object oldValue = maps[1].put(value, key);
+        if (oldValue != null) {
+            maps[0].remove(oldValue);
+        }
+        
         final Object obj = maps[0].put(key, value);
-        maps[1].put(value, key);
         return obj;
     }
 
     public Set entrySet() {
         // The entrySet is the root of most Map methods, care must be taken not 
         // to reference instance methods like size()
-        
+
         // Creates anonymous AbstractSet
         return new AbstractSet() {
-            
+
             public Iterator iterator() {
                 // Creates anonymous Iterator
                 return new Iterator() {
 
                     // Delegate iterator.
                     final Iterator it = maps[0].entrySet().iterator();
-                    
+
                     // Current iterator entry
                     Map.Entry currentEntry;
 
                     public void remove() {
                         // Removes from standard and inverse Maps.
-                        
+
                         // Object must be removed using the iterator or a 
                         // ConcurrentModificationException is thrown
                         it.remove();
@@ -168,7 +174,7 @@ public class HashBidiMap extends AbstractMap implements BidiMap {
             public int size() {
                 return HashBidiMap.this.maps[0].size();
             }
-            
+
         }; // anonymous AbstractSet
 
     } // entrySet()
@@ -225,8 +231,8 @@ public class HashBidiMap extends AbstractMap implements BidiMap {
                     }; // anonymous Iterator
                 }
 
-            };  // anonymous AbstractSet
+            }; // anonymous AbstractSet
-            
+
         } // entrySet()
 
     } // InverseBidiMap

src/test/org/apache/commons/collections/TestBidiMap.java

@@ -1,5 +1,5 @@
 /*
- * $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/test/org/apache/commons/collections/Attic/TestBidiMap.java,v 1.1 2003/09/23 20:29:34 matth Exp $
+ * $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/test/org/apache/commons/collections/Attic/TestBidiMap.java,v 1.2 2003/09/26 23:28:43 matth Exp $
  * ====================================================================
  *
  * The Apache Software License, Version 1.1
@@ -59,14 +59,16 @@ package org.apache.commons.collections;
 
 import java.util.Map;
 
+import junit.framework.TestCase;
+
 /**
  * JUnit tests.
  * 
  * @author Matthew Hawthorne
- * @version $Id: TestBidiMap.java,v 1.1 2003/09/23 20:29:34 matth Exp $
+ * @version $Id: TestBidiMap.java,v 1.2 2003/09/26 23:28:43 matth Exp $
  * @see org.apache.commons.collections.BidiMap
  */
-public abstract class TestBidiMap extends TestMap {
+public abstract class TestBidiMap extends TestCase {
 
     // Test data.
     private static final Object KEY = "key1";
@@ -130,7 +132,36 @@ public abstract class TestBidiMap extends TestMap {
             inverseMap.getKey(entries[0][0]));
     }
 
+    /**
+     * Ensures that calling:
+     * 
+     * <pre>
+     * map.add(a, c)
+     * map.add(b, c)
+     * </pre>
+     * 
+     * Removes the entry (a, c)
+     */
+    public void testAddDuplicateValue() {
+        final BidiMap map = createBidiMap();
+
+        final Object key1 = "key1";
+        final Object key2 = "key2";
+        final Object value = "value";
+
+        map.put(key1, value);
+        map.put(key2, value);
+
+        assertTrue(
+            "Key/value pair was not removed on duplicate value.",
+            !map.containsKey(key1));
+            
+        assertEquals("Key/value mismatch", key2, map.getKey(value));
+    }
+
+    // ----------------------------------------------------------------
     // Removal tests
+    // ----------------------------------------------------------------
 
     public void testClear() {
         BidiMap map = createBidiMapWithData();
@@ -153,7 +184,7 @@ public abstract class TestBidiMap extends TestMap {
     public void testRemove() {
         remove(createBidiMapWithData(), KEY);
         remove(createBidiMapWithData().inverseBidiMap(), VALUE);
-        
+
         removeKey(createBidiMapWithData(), VALUE);
         removeKey(createBidiMapWithData().inverseBidiMap(), KEY);
     }
@@ -163,7 +194,7 @@ public abstract class TestBidiMap extends TestMap {
         assertTrue("Key was not removed.", !map.containsKey(key));
         assertNull("Value was not removed.", map.getKey(value));
     }
-    
+
     private final void removeKey(BidiMap map, Object value) {
         final Object key = map.removeKey(value);
         assertTrue("Key was not removed.", !map.containsKey(key));
@@ -215,11 +246,15 @@ public abstract class TestBidiMap extends TestMap {
     // Data generation methods
     // ----------------------------------------------------------------
 
+    /**
+     * This classes used to extend collections.TestMap, but can't anymore since 
+     * put() breaks a contract.
+     */
     protected Map makeEmptyMap() {
         return createBidiMap();
     }
 
-    private final BidiMap createBidiMapWithData() {
+    protected final BidiMap createBidiMapWithData() {
         final BidiMap map = createBidiMap();
         fillMap(map);
         return map;