commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Vahrst (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (COLLECTIONS-441) MultiKeyMap.clone() should call super.clone()
Date Sat, 02 Feb 2013 21:52:12 GMT

     [ https://issues.apache.org/jira/browse/COLLECTIONS-441?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Thomas Vahrst updated COLLECTIONS-441:
--------------------------------------

    Description: 
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? 









  was:
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();
        m.put("A", "B", "C");
        
        MultiKeyMapSubclass m2 = (MultiKeyMapSubclass) m.clone();
        assertEquals("C", m.get("A", "B"));
        
    }



{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? 









    
> 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