Uploaded image for project: 'Commons Collections'
  1. Commons Collections
  2. COLLECTIONS-441

MultiKeyMap.clone() should call super.clone()

Attach filesAttach ScreenshotVotersWatch issueWatchersCreate sub-taskLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Improvement
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • None
    • 4.0-alpha1, 4.0
    • Map
    • None

    Description

      This issue addresses a findbugs issue:

      org.apache.commons.collections.map.MultiKeyMap.clone() does not call super.clone()

      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:

      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();
              
          }
      
      
      
      

      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.

      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...
              }    
          }
      
      
      

      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?

      Attachments

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            Unassigned Unassigned
            t.vahrst Thomas Vahrst
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment