[
https://issues.apache.org/jira/browse/COLLECTIONS-441?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13569649#comment-13569649
]
Thomas Vahrst commented on COLLECTIONS-441:
-------------------------------------------
In the suggested solution, the decorated map itself (an AbstractHashedMap
instance) is cloned and then filled into the new MultiKeyMap instance. The
clone() method of AbstractHashedMap contains another findbugs issue: it returns
null in the case of a CloneNotSupportedException:
{quote}
org.apache.commons.collections.map.AbstractHashedMap.clone() may return null
{quote}
Instead of returning null, throwing a RuntimeException makes findbugs happy ;-)
{code}
/**
* Clones the map without cloning the keys or values.
* <p>
* To implement <code>clone()</code>, a subclass must implement the
* <code>Cloneable</code> interface and make this method public.
*
* @return a shallow clone
*/
@Override
@SuppressWarnings("unchecked")
protected AbstractHashedMap<K, V> clone() {
try {
final AbstractHashedMap<K, V> cloned = (AbstractHashedMap<K, V>)
super.clone();
cloned.data = new HashEntry[data.length];
cloned.entrySet = null;
cloned.keySet = null;
cloned.values = null;
cloned.modCount = 0;
cloned.size = 0;
cloned.init();
cloned.putAll(this);
return cloned;
} catch (final CloneNotSupportedException ex) {
// return null; // should never happen.
throw new RuntimeException(ex); // should never happen.
}
}
{code}
> MultiKeyMap.clone() should call super.clone()
> ---------------------------------------------
>
> Key: COLLECTIONS-441
> URL: https://issues.apache.org/jira/browse/COLLECTIONS-441
> Project: Commons Collections
> Issue Type: Improvement
> Components: Map
> Affects Versions: 4.0-beta-1
> Reporter: Thomas Vahrst
> Priority: Minor
>
> This issue addresses a findbugs issue:
> {quote}
> org.apache.commons.collections.map.MultiKeyMap.clone() does not call
> super.clone()
> {quote}
> The current clone() implementation creates a new MultiKeyMap instance. This
> will lead to problems when clone() is invoked on subclasses of MultiKeyMap.
> This is a corresponding junit test which fails:
> {code}
> class MultiKeyMapTest
> // Subclass to test clone() method
> private static class MultiKeyMapSubclass extends MultiKeyMap<String,
> String>{
> }
> public void testCloneSubclass(){
> MultiKeyMapSubclass m = new MultiKeyMapSubclass();
>
> MultiKeyMapSubclass m2 = (MultiKeyMapSubclass) m.clone();
>
> }
> {code}
> Instead of creating a new MultiKeyMap instance, the clone() method should
> invoke super.clone() which leads in Object.clone(). This always returns an
> object of the correct type.
> {code}
> class MultiKeyMap{
> /**
> * Clones the map without cloning the keys or values.
> *
> * @return a shallow clone
> */
> @Override
> public MultiKeyMap<K, V> clone() {
> try {
> MultiKeyMap<K,V> m = (MultiKeyMap<K, V>) super.clone();
> AbstractHashedMap<MultiKey<? extends K>, V> del =
> (AbstractHashedMap<MultiKey<? extends K>, V>)decorated().clone();
>
> m.map = del;
> ((AbstractMapDecorator<K,V>)m).map = (Map) del;
> return m;
> } catch (CloneNotSupportedException ex) {
> throw new RuntimeException (ex); // this should never happen...
> }
> }
> {code}
> *Note*
> For serialisation compatibilty reasons to commons collections V.3.2,
> MultiKeyMap contains a map reference (the decorated map) which hides the same
> field in the super class AbstractMapDecorator. This is quite 'ugly' to
> understand and maintain.
> Should we consider to break the compatibility to the 3.2 version?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
