commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Neidhart (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (COLLECTIONS-441) MultiKeyMap.clone() should call super.clone()
Date Thu, 07 Feb 2013 13:53:12 GMT

    [ https://issues.apache.org/jira/browse/COLLECTIONS-441?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13573490#comment-13573490
] 

Thomas Neidhart commented on COLLECTIONS-441:
---------------------------------------------

Hi Thomas,

thanks for looking into this, just a few comments:

 * I would prefer to throw an InternalError in clone() to be consistent (also other maps do
this now)
 * removing the duplicate map makes sense, but I do not know yet if we should do it, bc with
3.x is broken
   in trunk (> 300 clirr errors), but supporting backwards compatible serialization may
be still wanted

this needs to be discussed on the mailinglist
                
> 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

Mime
View raw message