2

I am comparing every entry in a HashMap to every other entry in the same HashMap. While iterating the HashMap I remove some elements based on some conditions. However I keep getting the ConcurrentModificationException.

    Iterator<Entry<String, String>> i = map.entrySet().iterator();

    while (i.hasNext()) {
        Entry<String, String> next = i.next();

        for (Entry<String,String> e : map.entrySet()) {

            if (e.getKey() != next.getKey()){                 
              String[] positions_e = fields_e[1].split("-");
              int start_e = Integer.parseInt(positions_e[0]);
              int end_e =  Integer.parseInt(positions_e[1]);

              String[] positions_next = fields_next[1].split("-");
              int start_next = Integer.parseInt(positions_next[0]);
              int end_next =  Integer.parseInt(positions_next[1]);

            if (start_e <= start_next || end_e <= end_next )) { 
                 i.remove();        
           } 

       }

   }
user85421
  • 28,957
  • 10
  • 64
  • 87
sart
  • 33
  • 1
  • 9
  • 5
    Create a new collection of keys to be removed and instead of `i.remove(); ` do `toBeRemoved.add(i.getKey());`. Then outside of while loop `toBeRemoved.forEach(t -> map.remove(t));` – Boris Pavlović Feb 01 '19 at 12:37
  • 1
    There's no way to do that in the same hashmap. Create a replica of the exisiting hashmap and then remove elements in the duplicate. – sameera sy Feb 01 '19 at 12:38
  • @BorisPavlović I am using java 7 – sart Feb 01 '19 at 12:39
  • Unless this is not your real code, note that the positions, start and end calculations could be done before the loop... In which case the whole problem becomes much simpler. – assylias Feb 01 '19 at 12:40
  • 1
    @sart then iterate through `toBeRemoved` in a `for` loop... – Boris Pavlović Feb 01 '19 at 12:41
  • You could (or should) have `break;` after `i.remove();`. Not sure why you'd let the inner loop continue in that case – ernest_k Feb 01 '19 at 12:41
  • @assylias I am not sure how the start and end calculation could be done before the loop since I would like to do these calculations for each entry. – sart Feb 01 '19 at 12:48
  • 1
    @sart but you are not using `e` or `next` to calculate those values... – assylias Feb 01 '19 at 12:52
  • @assylias you are right – sart Feb 01 '19 at 12:59
  • @ernest_k it worked when I used the break; thank you! – sart Feb 01 '19 at 13:15
  • 1
    So then, after you’ve confirmed what @assylias said, what is the question? Inserting a `break` effectively turns the inner loop into code a block executed only one-time, which is possible as the inner code block does not depend on the loop variable, but in fact, it does not even need the outer loop. You only need to perform this calculation once, as its result never changes, and if `true`, call `clear()` on the `map`. But you should really think about how you came to the double-loop solution to evaluate an unchanging condition. There seems to be a huge logic error in your application. – Holger Feb 22 '19 at 08:19
  • 1
    Does this answer your question? [Iterating through a Collection, avoiding ConcurrentModificationException when removing objects in a loop](https://stackoverflow.com/questions/223918/iterating-through-a-collection-avoiding-concurrentmodificationexception-when-re) – Aswin Prasad Feb 25 '20 at 23:21

4 Answers4

1

Use iterator or lambda with removeIf for the second loop.

for (Entry<String,String> e : map.entrySet())

Iterator for map: https://www.techiedelight.com/iterate-map-in-java-using-entryset/

Lambda for map: https://javarevisited.blogspot.com/2017/08/how-to-remove-key-value-pairs-from-hashmap-java8-example.html#axzz5eHbDQxwp

singh30
  • 1,335
  • 17
  • 22
1

Here´s a method that does not involve a second map or list and also increases the readability of your code:

Extract your condition in a spearate method:

private boolean myCondition(Entry<String, String> currentEntry, Map<String, String> map) {
    for (Entry<String, String> entry : map.entrySet()) {
        ...
        if (...) {
            return true;
        }

        return false;
    }
}

Use java8 streams to filter the map according to your condition:

Map<String, String> filteredMap = map.entrySet().stream()
    .filter(entry -> myCondition(entry, map))
    .collect(Collectors.toMap(Entry::getKey, Entry::getValue));
Sebastian
  • 1,642
  • 13
  • 26
1

For the safety of your code, java does not allow you to remove elements that belong to your data structure while you are iterating it. A way of doing it so is: cloning your hashmap, iterating through the copy map and doing the comparison on it. if the condition shows that an element must be removed, try to remove it from your original hash map.

0

You're removing elements from the collection via the iterator i while iterating over it with a different iterator, the implied one used by your foreach loop.

You're always going to have this problem if you have nested iterations over the same collection and try to remove items from within the inner loop. Using either the inner or outer iterator to remove the element from the collection produces a ConcurrentModificationException from the other iterator.

Since you're using i to remove the element, the best strategy here is probably to break out of the inner loop after calling remove.

Willis Blackburn
  • 8,068
  • 19
  • 36