harmony-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From telli...@apache.org
Subject svn commit: r371027 - in /incubator/harmony/enhanced/classlib/trunk/modules/luni/src: main/java/java/util/IdentityHashMap.java test/java/org/apache/harmony/tests/java/util/AllTests.java test/java/org/apache/harmony/tests/java/util/IdentityHashMapTest.java
Date Sat, 21 Jan 2006 13:18:37 GMT
Author: tellison
Date: Sat Jan 21 05:18:29 2006
New Revision: 371027

URL: http://svn.apache.org/viewcvs?rev=371027&view=rev
Log:
Fixes for:
 - HARMONY-37 (remove() method of IdentityHashMap works incorrectly)
 - literal null not handled correctly as key/value
 - clear() now notes modification for iter'

Added:
    incubator/harmony/enhanced/classlib/trunk/modules/luni/src/test/java/org/apache/harmony/tests/java/util/IdentityHashMapTest.java
Modified:
    incubator/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/IdentityHashMap.java
    incubator/harmony/enhanced/classlib/trunk/modules/luni/src/test/java/org/apache/harmony/tests/java/util/AllTests.java

Modified: incubator/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/IdentityHashMap.java
URL: http://svn.apache.org/viewcvs/incubator/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/IdentityHashMap.java?rev=371027&r1=371026&r2=371027&view=diff
==============================================================================
--- incubator/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/IdentityHashMap.java
(original)
+++ incubator/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/IdentityHashMap.java
Sat Jan 21 05:18:29 2006
@@ -69,6 +69,12 @@
 	 * the Identityhashmap and the iterator
 	 */
 	transient int modCount = 0;
+	
+	/* Object used to represent null keys and values. 
+	 * This is used to differentiate a literal 'null' key
+	 * value pair from an empty spot in the map.
+	 */
+	private static final Object NULL_OBJECT = new Object();
 
 	static class IdentityHashMapEntry extends MapEntry {
 		IdentityHashMapEntry(Object theKey, Object theValue) {
@@ -121,11 +127,9 @@
 
 		public boolean hasNext() {
 			while (position < associatedMap.elementData.length) {
-				// check if position is an empty spot, not just an entry with
-				// null key
-				if (associatedMap.elementData[position] == null
-						&& associatedMap.elementData[position + 1] == null)
-					position = position + 2;
+				// if this is an empty spot, go to the next one
+				if (associatedMap.elementData[position] == null)
+					position += 2;
 				else
 					return true;
 			}
@@ -139,26 +143,23 @@
 
 		public Object next() {
 			checkConcurrentMod();
-			if (!hasNext())
-				throw new NoSuchElementException();
-
-			IdentityHashMapEntry result = new IdentityHashMapEntry(
-					associatedMap.elementData[position],
-					associatedMap.elementData[position + 1]);
+			if (!hasNext()) throw new NoSuchElementException();
+			
+			IdentityHashMapEntry result = associatedMap.getEntry(position);
 			lastPosition = position;
 			position += 2;
-
+			
 			canRemove = true;
 			return type.get(result);
 		}
 
 		public void remove() {
 			checkConcurrentMod();
-			if (!canRemove)
-				throw new IllegalStateException();
-
+			if (!canRemove) throw new IllegalStateException();
+			
 			canRemove = false;
 			associatedMap.remove(associatedMap.elementData[lastPosition]);
+			position = lastPosition;
 			expectedModCount++;
 		}
 	}
@@ -278,6 +279,7 @@
 		for (int i = 0; i < elementData.length; i++) {
 			elementData[i] = null;
 		}
+		modCount ++;
 	}
 
 	/**
@@ -288,13 +290,12 @@
 	 * @return true if <code>key</code> is a key of this Map, false otherwise
 	 */
 	public boolean containsKey(Object key) {
-		int index = findIndex(key, elementData);
-		if (key != null)
-			return elementData[index] == key;
-		// if key is null, we have to make sure
-		// what we found is not one of the empty spots
-		return (elementData[index] == null && elementData[index + 1] != null);
+		if (key == null) {
+			key = NULL_OBJECT;
+		}
 
+		int index = findIndex(key, elementData);
+		return elementData[index] == key;
 	}
 
 	/**
@@ -307,14 +308,13 @@
 	 *         otherwise
 	 */
 	public boolean containsValue(Object value) {
+		if (value == null) {
+			value = NULL_OBJECT;
+		}
+
 		for (int i = 1; i < elementData.length; i = i + 2) {
 			if (elementData[i] == value) {
-				if (value != null)
-					return true;
-				// if value is null, we have to make sure what we found is
-				// not one of the empty spots
-				if (elementData[i - 1] != null)
-					return true;
+				return true;
 			}
 		}
 		return false;
@@ -328,23 +328,52 @@
 	 * @return the value of the mapping with the specified key
 	 */
 	public Object get(Object key) {
+		if (key == null) {
+			key = NULL_OBJECT;
+		}
+
 		int index = findIndex(key, elementData);
 
-		if (elementData[index] == key)
-			return elementData[index + 1];
-		
+		if (elementData[index] == key) {
+			Object result = elementData[index + 1];
+			return (result == NULL_OBJECT) ? null : result;
+		}
+
 		return null;
 	}
 
 	private IdentityHashMapEntry getEntry(Object key) {
+		if (key == null) {
+			key = NULL_OBJECT;
+		}
+
 		int index = findIndex(key, elementData);
-		if (elementData[index] == key)
-			return new IdentityHashMapEntry(key, elementData[index + 1]);
-		
+		if (elementData[index] == key) {
+			return getEntry(index);
+		}
+
 		return null;
 	}
 
 	/**
+	 * Convenience method for getting the IdentityHashMapEntry without the
+	 * NULL_OBJECT elements
+	 */
+	private IdentityHashMapEntry getEntry(int index) {
+		Object key = elementData[index];
+		Object value = elementData[index + 1];
+
+		if (key == NULL_OBJECT) {
+			key = null;
+		}
+		if (value == NULL_OBJECT) {
+			value = null;
+		}
+
+		return new IdentityHashMapEntry(key, value);
+	}
+
+	/**
 	 * Returns the index where the key is found at, or the index of the next
 	 * empty spot if the key is not found in this table.
 	 */
@@ -353,9 +382,13 @@
 		int index = getModuloHash(key, length);
 		int last = (index + length - 2) % length;
 		while (index != last) {
-			if (array[index] == key
-					|| (array[index] == null && array[index + 1] == null))
+			if (array[index] == key || (array[index] == null)) {
+				/*
+				 * Found the key, or the next empty spot (which means key is not
+				 * in the table)
+				 */
 				break;
+			}
 			index = (index + 2) % length;
 		}
 		return index;
@@ -376,14 +409,18 @@
 	 *         if there was no mapping
 	 */
 	public Object put(Object key, Object value) {
+		if (key == null) {
+			key = NULL_OBJECT;
+		}
+
+		if (value == null) {
+			value = NULL_OBJECT;
+		}
+
 		int index = findIndex(key, elementData);
 
 		// if the key doesn't exist in the table
-		if (elementData[index] != key
-				|| (key == null && elementData[index + 1] == null)) {
-			// if key is null, and value is null
-			// this is one of the empty spots, there is not entry for key "null"
-			// in the table
+		if (elementData[index] != key) {
 			modCount++;
 			if (++size > threshold) {
 				rehash();
@@ -398,7 +435,8 @@
 		// insert value to where it needs to go, return the old value
 		Object result = elementData[index + 1];
 		elementData[index + 1] = value;
-		return result;
+
+		return (result == NULL_OBJECT) ? null : result;
 	}
 
 	private void rehash() {
@@ -408,9 +446,8 @@
 		Object[] newData = newElementArray(newlength);
 		for (int i = 0; i < elementData.length; i = i + 2) {
 			Object key = elementData[i];
-			if (key != null || (key == null && elementData[i + 1] != null)) { // if
-				// not
-				// empty
+			if (key != null) {
+				// if not empty
 				int index = findIndex(key, newData);
 				newData[index] = key;
 				newData[index + 1] = elementData[i + 1];
@@ -433,16 +470,20 @@
 	 *         this Map
 	 */
 	public Object remove(Object key) {
-		boolean hashedOk;
+		if (key == null) {
+			key = NULL_OBJECT;
+		}
 
+		boolean hashedOk;
 		int index, next, hash;
 		Object result, object;
 		index = next = findIndex(key, elementData);
 
+		if (elementData[index] != key)
+			return null;
+
 		// store the value for this key
 		result = elementData[index + 1];
-		if (result == null && key == null)
-			return null;
 
 		// shift the following elements up if needed
 		// until we reach an empty spot
@@ -450,7 +491,7 @@
 		while (true) {
 			next = (next + 2) % length;
 			object = elementData[next];
-			if (object == null && elementData[next + 1] == null)
+			if (object == null)
 				break;
 
 			hash = getModuloHash(object, length);
@@ -462,7 +503,7 @@
 			}
 			if (!hashedOk) {
 				elementData[index] = object;
-				elementData[index + 1] = elementData[next];
+				elementData[index + 1] = elementData[next + 1];
 				index = next;
 			}
 		}
@@ -474,7 +515,7 @@
 		elementData[index] = null;
 		elementData[index + 1] = null;
 
-		return result;
+		return (result == NULL_OBJECT) ? null : result;
 	}
 
 	/**
@@ -559,6 +600,17 @@
 							return entry.value;
 						}
 					}, IdentityHashMap.this);
+				}
+				
+				public boolean remove(Object object) {
+					Iterator it = iterator();
+					while (it.hasNext()) {
+						if (object == it.next()) {
+							it.remove();
+							return true;
+						}
+					}
+					return false;
 				}
 			};
 		}

Modified: incubator/harmony/enhanced/classlib/trunk/modules/luni/src/test/java/org/apache/harmony/tests/java/util/AllTests.java
URL: http://svn.apache.org/viewcvs/incubator/harmony/enhanced/classlib/trunk/modules/luni/src/test/java/org/apache/harmony/tests/java/util/AllTests.java?rev=371027&r1=371026&r2=371027&view=diff
==============================================================================
--- incubator/harmony/enhanced/classlib/trunk/modules/luni/src/test/java/org/apache/harmony/tests/java/util/AllTests.java
(original)
+++ incubator/harmony/enhanced/classlib/trunk/modules/luni/src/test/java/org/apache/harmony/tests/java/util/AllTests.java
Sat Jan 21 05:18:29 2006
@@ -30,6 +30,7 @@
 		//$JUnit-BEGIN$
 		suite.addTestSuite(ArraysTest.class);
 		suite.addTestSuite(CollectionsTest.class);
+		suite.addTestSuite(IdentityHashMapTest.class);
 		//$JUnit-END$
 		return suite;
 	}

Added: incubator/harmony/enhanced/classlib/trunk/modules/luni/src/test/java/org/apache/harmony/tests/java/util/IdentityHashMapTest.java
URL: http://svn.apache.org/viewcvs/incubator/harmony/enhanced/classlib/trunk/modules/luni/src/test/java/org/apache/harmony/tests/java/util/IdentityHashMapTest.java?rev=371027&view=auto
==============================================================================
--- incubator/harmony/enhanced/classlib/trunk/modules/luni/src/test/java/org/apache/harmony/tests/java/util/IdentityHashMapTest.java
(added)
+++ incubator/harmony/enhanced/classlib/trunk/modules/luni/src/test/java/org/apache/harmony/tests/java/util/IdentityHashMapTest.java
Sat Jan 21 05:18:29 2006
@@ -0,0 +1,60 @@
+/*
+ * Copyright 2006 The Apache Software Foundation or its licensors, as applicable
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.harmony.tests.java.util;
+
+import java.util.IdentityHashMap;
+
+import junit.framework.TestCase;
+
+public class IdentityHashMapTest extends TestCase {
+
+	/**
+	 * @tests java.util.IdentityHashMap#put(java.lang.Object, java.lang.Object)
+	 */
+	public void test_putLjava_lang_ObjectLjava_lang_Object() {
+		IdentityHashMap map = new IdentityHashMap();
+		
+		// Test null as a key.
+		Object value = "Some value";
+		map.put(null, value);
+		assertSame("Assert 0: Failure getting null key", value, map.get(null));
+		
+		// Test null as a value
+		Object key = "Some key";
+		map.put(key, null);
+		assertNull("Assert 1: Failure getting null value", map.get(key));
+	}
+
+	/**
+	 * @tests java.util.IdentityHashMapTest#remove(java.lang.Object)
+	 */
+	public void test_removeLjava_lang_Object() {
+		// Regression for HARMONY-37
+		IdentityHashMap hashMap = new IdentityHashMap();
+		hashMap.remove("absent");
+		assertEquals("Assert 0: Size is incorrect", 0, hashMap.size());
+
+		hashMap.put("key", "value");
+        hashMap.remove("key");
+        assertEquals("Assert 1: After removing non-null element size is incorrect", 0, hashMap.size());
+
+        hashMap.put(null, null);
+        assertEquals("Assert 2: adding literal null failed", 1, hashMap.size());
+        hashMap.remove(null);
+        assertEquals("Assert 3: After removing null element size is incorrect", 0, hashMap.size());
+	}
+}



Mime
View raw message