6

I have 2 HashMap<Integer,Point3D> objects names are positiveCoOrdinate and negativeCoOrdinates.

I am checking PositiveCoOrdinates with following condition.if it satisfies that corresponding point adding into negativeCoOrdinates and deleting from positiveCoOrdinates.

  HashMap<Integer, Point3d> positiveCoOrdinates=duelList.get(1);
  HashMap<Integer, Point3d> negativecoOrdinates=duelList.get(2);
  //condition
  Set<Integer> set=positiveCoOrdinates.keySet();
    for (Integer pointIndex : set) {
        Point3d coOrdinate=positiveCoOrdinates.get(pointIndex);
        if (coOrdinate.x>xMaxValue || coOrdinate.y>yMaxValue || coOrdinate.z>zMaxValue) {
            negativecoOrdinates.put(pointIndex, coOrdinate);
            positiveCoOrdinates.remove(pointIndex);
        }
    }

While adding,deleting time I am getting the following error.

 Exception in thread "main" java.util.ConcurrentModificationException
at java.util.HashMap$HashIterator.nextEntry(Unknown Source)
at java.util.HashMap$KeyIterator.next(Unknown Source)
at PlaneCoOrdinates.CoordinatesFiltering.Integration(CoordinatesFiltering.java:167)
at PlaneCoOrdinates.CoordinatesFiltering.main(CoordinatesFiltering.java:179)

For my testing,I mention System.out.println(coOrdinate.x); statement inside If condition.it's working fine.

If I add 2 lines(What I mention above) inside If condition,it throwing error.

How can I fix this.

Thanks.

Hanumath
  • 1,117
  • 9
  • 23
  • 41

4 Answers4

11

The easiest way is to make a copy of the keySet:

  Set<Integer> set= new HashSet<Integer>(positiveCoOrdinates.keySet());

The problem occurs because you are modifing the positiveCoOrdinates while you are using an Iterator that iterates through the keys.

You can also refactor your code and use an iterator over the entry set. This would be a better approach.

Set<Entry<Integer, Point3d>> entrySet = positiveCoOrdinates.entrySet();

    for (Iterator<Entry<Integer, Point3d>> iterator = entrySet.iterator(); iterator.hasNext();) {
        Entry<Integer, Point3d> entry = iterator.next();
        Point3d coOrdinate = entry.getValue();
        if (coOrdinate.x > xMaxValue || coOrdinate.y > yMaxValue
                || coOrdinate.z > zMaxValue) {
            Integer pointIndex = entry.getKey();
            negativecoOrdinates.put(pointIndex, coOrdinate);
            iterator.remove();
        }
    }
René Link
  • 48,224
  • 13
  • 108
  • 140
  • 3
    Or could use an `Iterator`. – Rahul Oct 01 '13 at 11:07
  • This worked, but as I'm using a sorted hash map I needed to use a new instance of LinkedHashSet to keep the order. I can't consider removing items using iterator.remove(), because I'm keeping track of auxiliar elements and swapping with it.next() when needed. Depending of the situation I should remove the auxiliar key instead of the current key retrieved from the it.next() method, then using a new instance of Set did the trick. I couldn't do this only using iterator.remove(). – daniel souza Apr 17 '16 at 23:47
2

You can't remove() from iterated collection when using the enhanced for-each loop. The for-each loop uses Iterator<Integer> implicitly. The JavaDoc clearly states that

The iterators returned by all of this class's "collection view methods" are fail-fast: if the map is structurally modified at any time after the iterator is created, in any way except through the iterator's own remove() method, the iterator will throw a ConcurrentModificationException. Thus, in the face of concurrent modification, the iterator fails quickly and cleanly, rather than risking arbitrary, non-deterministic behavior at an undetermined time in the future.

The for-each loop creates an iterator internally and uses it to traverse the set. Then you change the structure of the set ... and the iterator has to fail. The thing is that you don't have access to the iterator's methods, so you have to use Iterator<Integer> explicitly. The generated traversing bytecode will be the same, the only difference being you being able to remove elements from the list as you traverse it.

Set<Integer> set = positiveCoOrdinates.keySet();
for (Iterator<Integer> iterator = set.iterator(); iterator.hasNext(); ) {
    Integer pointIndex = iterator.next();
    Point3d coOrdinate = positiveCoOrdinates.get(pointIndex);
    if (coOrdinate.x>xMaxValue || coOrdinate.y>yMaxValue || coOrdinate.z>zMaxValue) {
        negativecoOrdinates.put(pointIndex, coOrdinate);
        iterator.remove(pointIndex);    // this line changed!
    }
}

If you are not familiar with iterators and their function, see the Oracle tutorial on Collections:

An Iterator is an object that enables you to traverse through a collection and to remove elements from the collection selectively, if desired. You get an Iterator for a collection by calling its iterator() method.

Note that Iterator.remove() is the only safe way to modify a collection during iteration; the behavior is unspecified if the underlying collection is modified in any other way while the iteration is in progress.

Use Iterator instead of the for-each construct when you need to:

  • Remove the current element. The for-each construct hides the iterator, so you cannot call remove(). Therefore, the for-each construct is not usable for filtering.
Petr Janeček
  • 37,768
  • 12
  • 121
  • 145
0

If you want to modify collection at run time, you need to use Iterator instead of enhanced for loop. Because enhanced for loop provide only read only functionality. Following is the Iterator example:

Iterator<Entity> iterator = collection.Iterator();
while(iterator.hasNext()){
  //DO Your Stuff
  iterator.remove(); // this function call remove the element from collection at run time
}
Harmeet Singh Taara
  • 6,483
  • 20
  • 73
  • 126
0

As it has been pointed out by René the reason for this very common issue is concurrent modification to a collection while it is being read by another.

You can use ConcurrentHashMap or collections like CopyOnWriteArrayLit, but beware that those approaches might be little bit expensive and simple changers to code to eliminate reading of the same collection while in a iteration will resolve this type of issues.

Upul Doluweera
  • 2,146
  • 1
  • 23
  • 28