You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by tn...@apache.org on 2013/04/28 20:52:51 UTC
svn commit: r1476813 - in /commons/proper/collections/trunk/src:
changes/changes.xml
main/java/org/apache/commons/collections4/map/Flat3Map.java
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();
}
}