commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t.@apache.org
Subject svn commit: r1476813 - in /commons/proper/collections/trunk/src: changes/changes.xml main/java/org/apache/commons/collections4/map/Flat3Map.java
Date Sun, 28 Apr 2013 18:52:51 GMT
Author: tn
Date: Sun Apr 28 18:52:50 2013
New Revision: 1476813

URL: http://svn.apache.org/r1476813
Log:
[COLLECTIONS-454] Fix findbugs warnings by returning independent Map.Entry objects from an
entrySet() iterator in Flat3Map.

Modified:
    commons/proper/collections/trunk/src/changes/changes.xml
    commons/proper/collections/trunk/src/main/java/org/apache/commons/collections4/map/Flat3Map.java

Modified: commons/proper/collections/trunk/src/changes/changes.xml
URL: http://svn.apache.org/viewvc/commons/proper/collections/trunk/src/changes/changes.xml?rev=1476813&r1=1476812&r2=1476813&view=diff
==============================================================================
--- commons/proper/collections/trunk/src/changes/changes.xml (original)
+++ commons/proper/collections/trunk/src/changes/changes.xml Sun Apr 28 18:52:50 2013
@@ -22,6 +22,11 @@
   <body>
 
   <release version="4.0" date="TBA" description="Next release">
+    <action issue="COLLECTIONS-454" dev="tn" type="update">
+      An iterator over a "Flat3Map#entrySet()" will now return
+      independent Map.Entry objects that will not change anymore when
+      the iterator progresses.
+    </action>
     <action issue="COLLECTIONS-452" dev="tn" type="update">
       Change base package to org.apache.commons.collections4.
     </action>

Modified: commons/proper/collections/trunk/src/main/java/org/apache/commons/collections4/map/Flat3Map.java
URL: http://svn.apache.org/viewvc/commons/proper/collections/trunk/src/main/java/org/apache/commons/collections4/map/Flat3Map.java?rev=1476813&r1=1476812&r2=1476813&view=diff
==============================================================================
--- commons/proper/collections/trunk/src/main/java/org/apache/commons/collections4/map/Flat3Map.java
(original)
+++ commons/proper/collections/trunk/src/main/java/org/apache/commons/collections4/map/Flat3Map.java
Sun Apr 28 18:52:50 2013
@@ -713,9 +713,10 @@ public class Flat3Map<K, V> implements I
     /**
      * Gets the entrySet view of the map.
      * Changes made to the view affect this map.
-     * The Map Entry is not an independent object and changes as the
-     * iterator progresses.
-     * To simply iterate through the entries, use {@link #mapIterator()}.
+     * <p>
+     * NOTE: from 4.0, the returned Map Entry will be an independent object and will
+     * not change anymore as the iterator progresses. To avoid this additional object
+     * creation and simply iterate through the entries, use {@link #mapIterator()}.
      *
      * @return the entrySet view
      */
@@ -771,45 +772,35 @@ public class Flat3Map<K, V> implements I
         }
     }
 
-    static abstract class EntryIterator<K, V> implements Map.Entry<K, V> {
+    static class FlatMapEntry<K, V> implements Map.Entry<K, V> {
         private final Flat3Map<K, V> parent;
-        private int nextIndex = 0;
-        protected boolean canRemove = false;
+        private final int index;
+        private volatile boolean removed;
 
-        /**
-         * Create a new Flat3Map.EntryIterator.
-         */
-        public EntryIterator(final Flat3Map<K, V> parent) {
+        public FlatMapEntry(final Flat3Map<K, V> parent, final int index) {
             this.parent = parent;
+            this.index = index;
+            this.removed = false;
         }
 
-        public boolean hasNext() {
-            return nextIndex < parent.size;
-        }
-
-        public Map.Entry<K, V> nextEntry() {
-            if (hasNext() == false) {
-                throw new NoSuchElementException(AbstractHashedMap.NO_NEXT_ENTRY);
-            }
-            canRemove = true;
-            nextIndex++;
-            return this;
-        }
-
-        public void remove() {
-            if (canRemove == false) {
-                throw new IllegalStateException(AbstractHashedMap.REMOVE_INVALID);
-            }
-            parent.remove(getKey());
-            nextIndex--;
-            canRemove = false;
+        /**
+         * Used by the iterator that created this entry to indicate that
+         * {@link java.util.Iterator#remove()} has been called.
+         * <p>
+         * As a consequence, all subsequent call to {@link #getKey()},
+         * {@link #setValue(Object)} and {@link #getValue()} will fail.
+         * 
+         * @param flag
+         */
+        void setRemoved(final boolean flag) {
+            this.removed = flag;
         }
 
         public K getKey() {
-            if (canRemove == false) {
+            if (removed) {
                 throw new IllegalStateException(AbstractHashedMap.GETKEY_INVALID);
             }
-            switch (nextIndex) {
+            switch (index) {
                 case 3:
                     return parent.key3;
                 case 2:
@@ -821,10 +812,10 @@ public class Flat3Map<K, V> implements I
         }
 
         public V getValue() {
-            if (canRemove == false) {
+            if (removed) {
                 throw new IllegalStateException(AbstractHashedMap.GETVALUE_INVALID);
             }
-            switch (nextIndex) {
+            switch (index) {
                 case 3:
                     return parent.value3;
                 case 2:
@@ -836,11 +827,11 @@ public class Flat3Map<K, V> implements I
         }
 
         public V setValue(final V value) {
-            if (canRemove == false) {
+            if (removed) {
                 throw new IllegalStateException(AbstractHashedMap.SETVALUE_INVALID);
             }
             final V old = getValue();
-            switch (nextIndex) {
+            switch (index) {
                 case 3:
                     parent.value3 = value;
                     break;
@@ -853,24 +844,10 @@ public class Flat3Map<K, V> implements I
             }
             return old;
         }
-    }
-
-    /**
-     * EntrySetIterator and MapEntry
-     */
-    static class EntrySetIterator<K, V> extends EntryIterator<K, V> implements
Iterator<Map.Entry<K, V>> {
-
-        EntrySetIterator(final Flat3Map<K, V> parent) {
-            super(parent);
-        }
-
-        public Map.Entry<K, V> next() {
-            return nextEntry();
-        }
 
         @Override
         public boolean equals(final Object obj) {
-            if (canRemove == false) {
+            if (removed) {
                 return false;
             }
             if (obj instanceof Map.Entry == false) {
@@ -885,7 +862,7 @@ public class Flat3Map<K, V> implements I
 
         @Override
         public int hashCode() {
-            if (canRemove == false) {
+            if (removed) {
                 return 0;
             }
             final Object key = getKey();
@@ -896,11 +873,61 @@ public class Flat3Map<K, V> implements I
 
         @Override
         public String toString() {
-            if (canRemove) {
+            if (!removed) {
                 return getKey() + "=" + getValue();
             }
             return "";
         }
+
+    }
+
+    static abstract class EntryIterator<K, V> {
+        private final Flat3Map<K, V> parent;
+        private int nextIndex = 0;
+        protected FlatMapEntry<K, V> currentEntry = null;
+
+        /**
+         * Create a new Flat3Map.EntryIterator.
+         */
+        public EntryIterator(final Flat3Map<K, V> parent) {
+            this.parent = parent;
+        }
+
+        public boolean hasNext() {
+            return nextIndex < parent.size;
+        }
+
+        public Map.Entry<K, V> nextEntry() {
+            if (!hasNext()) {
+                throw new NoSuchElementException(AbstractHashedMap.NO_NEXT_ENTRY);
+            }
+            currentEntry = new FlatMapEntry<K, V>(parent, ++nextIndex);
+            return currentEntry;
+        }
+
+        public void remove() {
+            if (currentEntry == null) {
+                throw new IllegalStateException(AbstractHashedMap.REMOVE_INVALID);
+            }
+            currentEntry.setRemoved(true);
+            parent.remove(currentEntry.getKey());
+            nextIndex--;
+            currentEntry = null;
+        }
+
+    }
+
+    /**
+     * EntrySetIterator and MapEntry
+     */
+    static class EntrySetIterator<K, V> extends EntryIterator<K, V> implements
Iterator<Map.Entry<K, V>> {
+        EntrySetIterator(final Flat3Map<K, V> parent) {
+            super(parent);
+        }
+
+        public Map.Entry<K, V> next() {
+            return nextEntry();
+        }
     }
 
     /**
@@ -973,8 +1000,7 @@ public class Flat3Map<K, V> implements I
         }
 
         public K next() {
-            nextEntry();
-            return getKey();
+            return nextEntry().getKey();
         }
     }
 
@@ -1041,8 +1067,7 @@ public class Flat3Map<K, V> implements I
         }
 
         public V next() {
-            nextEntry();
-            return getValue();
+            return nextEntry().getValue();
         }
     }
 



Mime
View raw message