scolebourne 2004/03/13 04:43:43
Modified: collections RELEASE-NOTES.html
collections/src/java/org/apache/commons/collections/keyvalue
MultiKey.java
collections/src/test/org/apache/commons/collections/keyvalue
TestMultiKey.java
Log:
MultiKey enhancements to add getKey(index) and size()
Make protected constructor public
Add lots of javadoc examples and warnings
Revision Changes Path
1.11 +13 -2 jakarta-commons/collections/RELEASE-NOTES.html
Index: RELEASE-NOTES.html
===================================================================
RCS file: /home/cvs/jakarta-commons/collections/RELEASE-NOTES.html,v
retrieving revision 1.10
retrieving revision 1.11
diff -u -r1.10 -r1.11
--- RELEASE-NOTES.html 27 Feb 2004 00:22:09 -0000 1.10
+++ RELEASE-NOTES.html 13 Mar 2004 12:43:43 -0000 1.11
@@ -34,7 +34,18 @@
<hr />
+<center><h3>BUG FIXES</h3></center>
+<p>
+[27159] AbstractHashedMap subclasses failed to clone() correctly
+</p>
+
+<center><h3>ENHANCEMENTS</h3></center>
+<p>
+MultiKey - Add getKey(index) and size() methods and make constructor public
+</p>
+
<center><h3>CHANGES</h3></center>
<p>
-[26470] Javadoc - Add javadoc about requiring Comparable entries
-[27159] Bug - AbstractHashedMap subclasses failed to clone() correctly
+[26470] TreeBidiMap - Add javadoc about requiring Comparable entries
+MultiKey - Add extra explanatations, examples and warnings
+</p>
1.5 +86 -15
jakarta-commons/collections/src/java/org/apache/commons/collections/keyvalue/MultiKey.java
Index: MultiKey.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/collections/src/java/org/apache/commons/collections/keyvalue/MultiKey.java,v
retrieving revision 1.4
retrieving revision 1.5
diff -u -r1.4 -r1.5
--- MultiKey.java 18 Feb 2004 01:00:08 -0000 1.4
+++ MultiKey.java 13 Mar 2004 12:43:43 -0000 1.5
@@ -25,6 +25,18 @@
* maps of maps. An example might be the need to lookup a filename by
* key and locale. The typical solution might be nested maps. This class
* can be used instead by creating an instance passing in the key and locale.
+ * <p>
+ * Example usage:
+ * <pre>
+ * // populate map with data mapping key+locale to localizedText
+ * Map map = new HashMap();
+ * MultiKey multiKey = new MultiKey(key, locale);
+ * map.put(multiKey, localizedText);
+ *
+ * // later retireve the localized text
+ * MultiKey multiKey = new MultiKey(key, locale);
+ * String localizedText = (String) map.get(multiKey);
+ * </pre>
*
* @since Commons Collections 3.0
* @version $Revision$ $Date$
@@ -33,6 +45,7 @@
* @author Stephen Colebourne
*/
public class MultiKey implements Serializable {
+ // This class could implement List, but that would confuse it's purpose
/** Serialisation version */
private static final long serialVersionUID = 4465448607415788805L;
@@ -44,6 +57,9 @@
/**
* Constructor taking two keys.
+ * <p>
+ * The keys should be immutable
+ * If they are not then they must not be changed after adding to the MultiKey.
*
* @param key1 the first key
* @param key2 the second key
@@ -54,6 +70,9 @@
/**
* Constructor taking three keys.
+ * <p>
+ * The keys should be immutable
+ * If they are not then they must not be changed after adding to the MultiKey.
*
* @param key1 the first key
* @param key2 the second key
@@ -65,6 +84,9 @@
/**
* Constructor taking four keys.
+ * <p>
+ * The keys should be immutable
+ * If they are not then they must not be changed after adding to the MultiKey.
*
* @param key1 the first key
* @param key2 the second key
@@ -77,6 +99,9 @@
/**
* Constructor taking five keys.
+ * <p>
+ * The keys should be immutable
+ * If they are not then they must not be changed after adding to the MultiKey.
*
* @param key1 the first key
* @param key2 the second key
@@ -89,9 +114,14 @@
}
/**
- * Constructor taking an array of keys.
+ * Constructor taking an array of keys which is cloned.
+ * <p>
+ * The keys should be immutable
+ * If they are not then they must not be changed after adding to the MultiKey.
+ * <p>
+ * This is equivalent to <code>new MultiKey(keys, true)</code>.
*
- * @param keys the array of keys
+ * @param keys the array of keys, not null
* @throws IllegalArgumentException if the key array is null
*/
public MultiKey(Object[] keys) {
@@ -99,20 +129,35 @@
}
/**
- * Constructor taking an array of keys.
+ * Constructor taking an array of keys, optionally choosing whether to clone.
* <p>
- * If the array is not copied, then it must not be modified.
+ * <b>If the array is not cloned, then it must not be modified.</b>
+ * <p>
+ * This method is public for performance reasons only, to avoid a clone.
+ * The hashcode is calculated once here in this method.
+ * Therefore, changing the array passed in would not change the hashcode but
+ * would change the equals method, which is a bug.
+ * <p>
+ * This is the only fully safe usage of this constructor, as the object array
+ * is never made available in a variable:
+ * <pre>
+ * new MultiKey(new Object[] {...}, false);
+ * </pre>
+ * <p>
+ * The keys should be immutable
+ * If they are not then they must not be changed after adding to the MultiKey.
*
- * @param keys the array of keys
- * @param makeCopy true to copy the array, false to assign it
+ * @param keys the array of keys, not null
+ * @param makeClone true to clone the array, false to assign it
* @throws IllegalArgumentException if the key array is null
+ * @since Commons Collections 3.1
*/
- protected MultiKey(Object[] keys, boolean makeCopy) {
+ public MultiKey(Object[] keys, boolean makeClone) {
super();
if (keys == null) {
throw new IllegalArgumentException("The array of keys must not be
null");
}
- if (makeCopy) {
+ if (makeClone) {
this.keys = (Object[]) keys.clone();
} else {
this.keys = keys;
@@ -121,18 +166,18 @@
int total = 0;
for (int i = 0; i < keys.length; i++) {
if (keys[i] != null) {
- if (i == 0) {
- total = keys[i].hashCode();
- } else {
- total ^= keys[i].hashCode();
- }
+ total ^= keys[i].hashCode();
}
}
hashCode = total;
}
+ //-----------------------------------------------------------------------
/**
- * Gets a copy of the individual keys.
+ * Gets a clone of the array of keys.
+ * <p>
+ * The keys should be immutable
+ * If they are not then they must not be changed.
*
* @return the individual keys
*/
@@ -140,6 +185,32 @@
return (Object[]) keys.clone();
}
+ /**
+ * Gets the key at the specified index.
+ * <p>
+ * The key should be immutable.
+ * If it is not then it must not be changed.
+ *
+ * @param index the index to retrieve
+ * @return the key at the index
+ * @throws IndexOutOfBoundsException if the index is invalid
+ * @since Commons Collections 3.1
+ */
+ public Object getKey(int index) {
+ return keys[index];
+ }
+
+ /**
+ * Gets the size of the list of keys.
+ *
+ * @return the size of the list of keys
+ * @since Commons Collections 3.1
+ */
+ public int size() {
+ return keys.length;
+ }
+
+ //-----------------------------------------------------------------------
/**
* Compares this object to another.
* <p>
1.4 +104 -5
jakarta-commons/collections/src/test/org/apache/commons/collections/keyvalue/TestMultiKey.java
Index: TestMultiKey.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/collections/src/test/org/apache/commons/collections/keyvalue/TestMultiKey.java,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -r1.3 -r1.4
--- TestMultiKey.java 18 Feb 2004 01:20:40 -0000 1.3
+++ TestMultiKey.java 13 Mar 2004 12:43:43 -0000 1.4
@@ -58,8 +58,8 @@
super.tearDown();
}
-
- public void testConstructorsAndGet() throws Exception {
+ //-----------------------------------------------------------------------
+ public void testConstructors() throws Exception {
MultiKey mk = null;
mk = new MultiKey(ONE, TWO);
Assert.assertTrue(Arrays.equals(new Object[] {ONE, TWO}, mk.getKeys()));
@@ -75,13 +75,109 @@
mk = new MultiKey(new Object[] {THREE, FOUR, ONE, TWO}, false);
Assert.assertTrue(Arrays.equals(new Object[] {THREE, FOUR, ONE, TWO},
mk.getKeys()));
-
- // don't do this!
+ }
+
+ public void testConstructorsByArray() throws Exception {
+ MultiKey mk = null;
Object[] keys = new Object[] {THREE, FOUR, ONE, TWO};
mk = new MultiKey(keys);
Assert.assertTrue(Arrays.equals(new Object[] {THREE, FOUR, ONE, TWO},
mk.getKeys()));
keys[3] = FIVE; // no effect
Assert.assertTrue(Arrays.equals(new Object[] {THREE, FOUR, ONE, TWO},
mk.getKeys()));
+
+ keys = new Object[] {};
+ mk = new MultiKey(keys);
+ Assert.assertTrue(Arrays.equals(new Object[] {}, mk.getKeys()));
+
+ keys = new Object[] {THREE, FOUR, ONE, TWO};
+ mk = new MultiKey(keys, true);
+ Assert.assertTrue(Arrays.equals(new Object[] {THREE, FOUR, ONE, TWO},
mk.getKeys()));
+ keys[3] = FIVE; // no effect
+ Assert.assertTrue(Arrays.equals(new Object[] {THREE, FOUR, ONE, TWO},
mk.getKeys()));
+
+ keys = new Object[] {THREE, FOUR, ONE, TWO};
+ mk = new MultiKey(keys, false);
+ Assert.assertTrue(Arrays.equals(new Object[] {THREE, FOUR, ONE, TWO},
mk.getKeys()));
+ // change key - don't do this!
+ // the hashcode of the MultiKey is now broken
+ keys[3] = FIVE;
+ Assert.assertTrue(Arrays.equals(new Object[] {THREE, FOUR, ONE, FIVE},
mk.getKeys()));
+ }
+
+ public void testConstructorsByArrayNull() throws Exception {
+ Object[] keys = null;
+ try {
+ new MultiKey(keys);
+ fail();
+ } catch (IllegalArgumentException ex) {}
+ try {
+ new MultiKey(keys, true);
+ fail();
+ } catch (IllegalArgumentException ex) {}
+ try {
+ new MultiKey(keys, false);
+ fail();
+ } catch (IllegalArgumentException ex) {}
+ }
+
+ public void testSize() {
+ Assert.assertEquals(2, new MultiKey(ONE, TWO).size());
+ Assert.assertEquals(2, new MultiKey(null, null).size());
+ Assert.assertEquals(3, new MultiKey(ONE, TWO, THREE).size());
+ Assert.assertEquals(3, new MultiKey(null, null, null).size());
+ Assert.assertEquals(4, new MultiKey(ONE, TWO, THREE, FOUR).size());
+ Assert.assertEquals(4, new MultiKey(null, null, null, null).size());
+ Assert.assertEquals(5, new MultiKey(ONE, TWO, THREE, FOUR, FIVE).size());
+ Assert.assertEquals(5, new MultiKey(null, null, null, null, null).size());
+
+ Assert.assertEquals(0, new MultiKey(new Object[] {}).size());
+ Assert.assertEquals(1, new MultiKey(new Object[] {ONE}).size());
+ Assert.assertEquals(2, new MultiKey(new Object[] {ONE, TWO}).size());
+ Assert.assertEquals(7, new MultiKey(new Object[] {ONE, TWO, ONE, TWO, ONE,
TWO, ONE}).size());
+ }
+
+ public void testGetIndexed() {
+ MultiKey mk = new MultiKey(ONE, TWO);
+ Assert.assertSame(ONE, mk.getKey(0));
+ Assert.assertSame(TWO, mk.getKey(1));
+ try {
+ mk.getKey(-1);
+ fail();
+ } catch (IndexOutOfBoundsException ex) {}
+ try {
+ mk.getKey(2);
+ fail();
+ } catch (IndexOutOfBoundsException ex) {}
+ }
+
+ public void testGetKeysSimpleConstructor() {
+ MultiKey mk = new MultiKey(ONE, TWO);
+ Object[] array = mk.getKeys();
+ Assert.assertSame(ONE, array[0]);
+ Assert.assertSame(TWO, array[1]);
+ Assert.assertEquals(2, array.length);
+ }
+
+ public void testGetKeysArrayConstructorCloned() {
+ Object[] keys = new Object[] {ONE, TWO};
+ MultiKey mk = new MultiKey(keys, true);
+ Object[] array = mk.getKeys();
+ Assert.assertTrue(array != keys);
+ Assert.assertTrue(Arrays.equals(array, keys));
+ Assert.assertSame(ONE, array[0]);
+ Assert.assertSame(TWO, array[1]);
+ Assert.assertEquals(2, array.length);
+ }
+
+ public void testGetKeysArrayConstructorNonCloned() {
+ Object[] keys = new Object[] {ONE, TWO};
+ MultiKey mk = new MultiKey(keys, false);
+ Object[] array = mk.getKeys();
+ Assert.assertTrue(array != keys); // still not equal
+ Assert.assertTrue(Arrays.equals(array, keys));
+ Assert.assertSame(ONE, array[0]);
+ Assert.assertSame(TWO, array[1]);
+ Assert.assertEquals(2, array.length);
}
public void testHashCode() {
@@ -92,6 +188,9 @@
Assert.assertTrue(mk1.hashCode() == mk1.hashCode());
Assert.assertTrue(mk1.hashCode() == mk2.hashCode());
Assert.assertTrue(mk1.hashCode() != mk3.hashCode());
+
+ int total = (0 ^ ONE.hashCode()) ^ TWO.hashCode();
+ Assert.assertEquals(total, mk1.hashCode());
}
public void testEquals() {
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
