You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by sc...@apache.org on 2004/06/12 01:27:38 UTC
cvs commit: jakarta-commons/collections/src/java/org/apache/commons/collections/bidimap AbstractDualBidiMap.java DualHashBidiMap.java DualTreeBidiMap.java
scolebourne 2004/06/11 16:27:38
Modified: collections RELEASE-NOTES.html
collections/src/java/org/apache/commons/collections/bidimap
AbstractDualBidiMap.java DualHashBidiMap.java
DualTreeBidiMap.java
Log:
Fix bug in DualBidiMaps caused by bad design of createMap method
bug 29519
Revision Changes Path
1.62 +1 -0 jakarta-commons/collections/RELEASE-NOTES.html
Index: RELEASE-NOTES.html
===================================================================
RCS file: /home/cvs/jakarta-commons/collections/RELEASE-NOTES.html,v
retrieving revision 1.61
retrieving revision 1.62
diff -u -r1.61 -r1.62
--- RELEASE-NOTES.html 10 Jun 2004 22:14:59 -0000 1.61
+++ RELEASE-NOTES.html 11 Jun 2004 23:27:37 -0000 1.62
@@ -104,6 +104,7 @@
<li>UnmodifiableSortedBag - Fix to ensure unmodifiable</li>
<li>MultiHashMap - Fix copy constructor and clone to work properly [28972]</li>
<li>ListOrderedSet - Fix to throw IllegalArgumentException instead of NPE on null factory decorate(List)</li>
+<li>*Dual*BidiMap - Fix poorly designed subclass method call from superclass constructor [29519]</li>
</ul>
<center><h3>JAVADOC</h3></center>
1.13 +30 -4 jakarta-commons/collections/src/java/org/apache/commons/collections/bidimap/AbstractDualBidiMap.java
Index: AbstractDualBidiMap.java
===================================================================
RCS file: /home/cvs/jakarta-commons/collections/src/java/org/apache/commons/collections/bidimap/AbstractDualBidiMap.java,v
retrieving revision 1.12
retrieving revision 1.13
diff -u -r1.12 -r1.13
--- AbstractDualBidiMap.java 15 May 2004 12:13:03 -0000 1.12
+++ AbstractDualBidiMap.java 11 Jun 2004 23:27:37 -0000 1.13
@@ -68,7 +68,9 @@
/**
* Creates an empty map, initialised by <code>createMap</code>.
* <p>
- * The map array must be populated by the subclass.
+ * This constructor remains in place for deserialization.
+ * All other usage is deprecated in favour of
+ * {@link #AbstractDualBidiMap(Map, Map)}.
*/
protected AbstractDualBidiMap() {
super();
@@ -76,6 +78,25 @@
maps[1] = createMap();
}
+ /**
+ * Creates an empty map using the two maps specified as storage.
+ * <p>
+ * The two maps must be a matching pair, normal and reverse.
+ * They will typically both be empty.
+ * <p>
+ * Neither map is validated, so nulls may be passed in.
+ * If you choose to do this then the subclass constructor must populate
+ * the <code>maps[]</code> instance variable itself.
+ *
+ * @param normalMap the normal direction map
+ * @param reverseMap the reverse direction map
+ */
+ protected AbstractDualBidiMap(Map normalMap, Map reverseMap) {
+ super();
+ maps[0] = normalMap;
+ maps[1] = reverseMap;
+ }
+
/**
* Constructs a map that decorates the specified maps,
* used by the subclass <code>createBidiMap</code> implementation.
@@ -94,11 +115,16 @@
/**
* Creates a new instance of the map used by the subclass to store data.
* <p>
- * Do not change any instance variables from this method.
+ * This design is deeply flawed and has been deprecated.
+ * It relied on subclass data being used during a superclass constructor.
*
* @return the map to be used for internal storage
+ * @deprecated For constructors, use the new two map constructor.
+ * For deserialization, populate the maps array directly in readObject.
*/
- protected abstract Map createMap();
+ protected Map createMap() {
+ return null;
+ }
/**
* Creates a new instance of the subclass.
1.7 +14 -13 jakarta-commons/collections/src/java/org/apache/commons/collections/bidimap/DualHashBidiMap.java
Index: DualHashBidiMap.java
===================================================================
RCS file: /home/cvs/jakarta-commons/collections/src/java/org/apache/commons/collections/bidimap/DualHashBidiMap.java,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -r1.6 -r1.7
--- DualHashBidiMap.java 18 Feb 2004 00:57:39 -0000 1.6
+++ DualHashBidiMap.java 11 Jun 2004 23:27:37 -0000 1.7
@@ -26,6 +26,14 @@
/**
* Implementation of <code>BidiMap</code> that uses two <code>HashMap</code> instances.
+ * <p>
+ * Two <code>HashMap</code> instances are used in this class.
+ * This provides fast lookups at the expense of storing two sets of map entries.
+ * Commons Collections would welcome the addition of a direct hash-based
+ * implementation of the <code>BidiMap</code> interface.
+ * <p>
+ * NOTE: From Commons Collections 3.1, all subclasses will use <code>HashMap</code>
+ * and the flawed <code>createMap</code> method is ignored.
*
* @since Commons Collections 3.0
* @version $Id$
@@ -40,10 +48,10 @@
private static final long serialVersionUID = 721969328361808L;
/**
- * Creates an empty <code>HashBidiMap</code>
+ * Creates an empty <code>HashBidiMap</code>.
*/
public DualHashBidiMap() {
- super();
+ super(new HashMap(), new HashMap());
}
/**
@@ -53,7 +61,7 @@
* @param map the map whose mappings are to be placed in this map
*/
public DualHashBidiMap(Map map) {
- super();
+ super(new HashMap(), new HashMap());
putAll(map);
}
@@ -69,15 +77,6 @@
}
/**
- * Creates a new instance of the map used by the subclass to store data.
- *
- * @return the map to be used for internal storage
- */
- protected Map createMap() {
- return new HashMap();
- }
-
- /**
* Creates a new instance of this object.
*
* @param normalMap the normal direction map
@@ -98,6 +97,8 @@
private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
in.defaultReadObject();
+ maps[0] = new HashMap();
+ maps[1] = new HashMap();
Map map = (Map) in.readObject();
putAll(map);
}
1.14 +10 -14 jakarta-commons/collections/src/java/org/apache/commons/collections/bidimap/DualTreeBidiMap.java
Index: DualTreeBidiMap.java
===================================================================
RCS file: /home/cvs/jakarta-commons/collections/src/java/org/apache/commons/collections/bidimap/DualTreeBidiMap.java,v
retrieving revision 1.13
retrieving revision 1.14
diff -u -r1.13 -r1.14
--- DualTreeBidiMap.java 15 May 2004 12:13:03 -0000 1.13
+++ DualTreeBidiMap.java 11 Jun 2004 23:27:37 -0000 1.14
@@ -44,6 +44,9 @@
* When considering whether to use this class, the {@link TreeBidiMap} class should
* also be considered. It implements the interface using a dedicated design, and does
* not store each object twice, which can save on memory use.
+ * <p>
+ * NOTE: From Commons Collections 3.1, all subclasses will use <code>TreeMap</code>
+ * and the flawed <code>createMap</code> method is ignored.
*
* @since Commons Collections 3.0
* @version $Id$
@@ -63,7 +66,7 @@
* Creates an empty <code>DualTreeBidiMap</code>
*/
public DualTreeBidiMap() {
- super();
+ super(new TreeMap(), new TreeMap());
this.comparator = null;
}
@@ -74,7 +77,7 @@
* @param map the map whose mappings are to be placed in this map
*/
public DualTreeBidiMap(Map map) {
- super();
+ super(new TreeMap(), new TreeMap());
putAll(map);
this.comparator = null;
}
@@ -85,12 +88,12 @@
* @param comparator the Comparator
*/
public DualTreeBidiMap(Comparator comparator) {
- super();
+ super(new TreeMap(comparator), new TreeMap(comparator));
this.comparator = comparator;
}
/**
- * Constructs a <code>HashBidiMap</code> that decorates the specified maps.
+ * Constructs a <code>DualTreeBidiMap</code> that decorates the specified maps.
*
* @param normalMap the normal direction map
* @param reverseMap the reverse direction map
@@ -100,15 +103,6 @@
super(normalMap, reverseMap, inverseBidiMap);
this.comparator = ((SortedMap) normalMap).comparator();
}
-
- /**
- * Creates a new instance of the map used by the subclass to store data.
- *
- * @return the map to be used for internal storage
- */
- protected Map createMap() {
- return new TreeMap(comparator);
- }
/**
* Creates a new instance of this object.
@@ -345,6 +339,8 @@
private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
in.defaultReadObject();
+ maps[0] = new TreeMap(comparator);
+ maps[1] = new TreeMap(comparator);
Map map = (Map) in.readObject();
putAll(map);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org