Can I use keySet to modify HashMap during iteration?

Question

I know that collections shouldn't be modified during iteration. So we should have workaround.

I have a code:

Map<Key, Value> map = getMap(); // map generating is hidden
for (Key key : map.keySet()) {
  if (isToRemove(key)) {
    map.remove(key);
  } else {
    map.put(key, getNewValue());
  }
}

Is it undefined behavior or valid code?

keySet documentation sais that changes of the map are reflected in returned set and vice-versa. Does it mean that previous code is unacceptable?

What you are asking seems unclear to me. You are generating a map, and want to remove keys after? why not prevent adding keys rather than adding them and remove them? — Kepotx, Apr 06 '18 at 08:01
What happens when you run this code? What's the purpose of your question, quiz us? — Abhijit Sarkar, Apr 06 '18 at 08:02
I would prefer to put all keys that should be removed in a set and after the loop I would loop through the set and remove the keys from the map. — Ralf Renz, Apr 06 '18 at 08:04
You have the answer in here https://stackoverflow.com/questions/6092642/how-to-remove-a-key-from-hashmap-while-iterating-over-it — kimy82, Apr 06 '18 at 08:05
@AbhijitSarkar The code works. But I'm not sure that it works always correctly. I know there are situations when error appears only in specific situations. That's my question about. Am I clear now? — Nimtar, Apr 06 '18 at 12:57
@Nimtar `Map` doesn't have `add()` method. I suppose that you would write `put()`. — davidxxx, Apr 08 '18 at 07:50
@davidxxx, yes, my fault. I couldn't use "copy and paste", so had the mistake. Thank you — Nimtar, Apr 08 '18 at 17:45

score 7 · Answer 1 · edited Sep 18 '21 at 07:39

The answer from davidxxx is correct (+1) in pointing out that the view collections on the Map are linked to the map, and that modifications to the map while iterating a view collection may result in ConcurrentModificationException. The view collections on a map are provided by the entrySet, keySet, and values methods.

Thus, the original code:

    Map<Key, Value> map = getMap();
    for (Key key : map.keySet()) {
        if (isToRemove(key)) {
            map.remove(key);
        } else {
            map.add(key, getNewValue());
        }
    }

will most likely throw ConcurrentModificationException because it modifies the map during each iteration.

It's possible to remove entries from the map while iterating a view collection, if that view collection's iterator supports the remove operation. The iterators for HashMap's view collections do support this. It is also possible to set the value of a particular map entry (key-value pair) by using the setValue method of a Map.Entry instance obtained while iterating a map's entrySet. Thus, it's possible to do what you want to do within a single iteration, without using a temporary map. Here's the code to do that:

    Map<Key, Value> map = getMap();
    for (var entryIterator = map.entrySet().iterator(); entryIterator.hasNext(); ) {
        var entry = entryIterator.next();
        if (isToRemove(entry.getKey())) {
            entryIterator.remove();
        } else {
            entry.setValue(getNewValue());
        }
    }

Note the use of Java 10's var construct. If you're not on Java 10, you have to write out the type declarations explicitly:

    Map<Key, Value> map = getMap();
    for (Iterator<Map.Entry<Key, Value>> entryIterator = map.entrySet().iterator(); entryIterator.hasNext(); ) {
        Map.Entry<Key, Value> entry = entryIterator.next();
        if (isToRemove(entry.getKey())) {
            entryIterator.remove();
        } else {
            entry.setValue(getNewValue());
        }
    }

Finally, given that this is a moderately complicated map operation, it might be fruitful to use a stream to do the work. Note that this creates a new map instead of modifying an existing map in-place.

    import java.util.Map.Entry;
    import static java.util.Map.entry; // requires Java 9

    Map<Key, Value> result =
        getMap().entrySet().stream()
                .filter(e -> ! isToRemove(e.getKey()))
                .map(e -> entry(e.getKey(), getNewValue()))
                .collect(toMap(Entry::getKey, Entry::getValue));

`Entry.setValue()` was really the thing to use in this case as the actual code didn't add but modified a new element in the map. — davidxxx, Apr 08 '18 at 07:53

davidxxx · Accepted Answer · 2018-04-08T08:03:54.177

The HashMap.keySet() method states more precisely:

The set is backed by the map, so changes to the map are reflected in the set, and vice-versa.

It means that the elements returned by keySet() and the keys of the Map refer to the same objects. So of course changing the state of any element of the Set (such as key.setFoo(new Foo());) will be reflected in the Map keys and reversely.

You should be cautious and prevent the map from being modified during the keyset() iteration :

If the map is modified while an iteration over the set is in progress (except through the iterator's own remove operation), the results of the iteration are undefined

You can remove entries of the map as :

The set supports element removal, which removes the corresponding mapping from the map, via the Iterator.remove, Set.remove, removeAll, retainAll, and clear operations.

But you cannot add entries in :

It does not support the add or addAll operations.

So in conclusion, during keySet() iterator use Set.remove() or more simply iterate with the Iterator of the keySet and invoke Iterator.remove() to remove elements from the map.
You can add new elements in a temporary Map that you will use after the iteration to populate the original Map.

For example :

Map<Key, Value> map = getMap(); // map generating is hidden

Map<Key, Value> tempMap = new HashMap<>();
for (Iterator<Key> keyIterator = map.keySet().iterator(); keyIterator.hasNext();) {
    Key key = keyIterator.next();
    if (isToRemove(key)) {
        keyIterator.remove();
    }
    else {
        tempMap.put(key, getNewValue());
    }
}

map.putAll(tempMap);

Edit : Note that as you want to modify existing entries of the map, you should use an Map.EntrySet as explained in the Stuart Marks answer.
In other cases, using an intermediary Map or a Stream that creates a new Map is required.

You can only use `Iterator.remove()` if you're iterating. Otherwise you will get a `ConcurrentModificationException`. — user207421, Apr 08 '18 at 05:22

score 1 · Answer 3 · answered Apr 06 '18 at 08:05

If you run your code you get a ConcurrentModificationException. Here is how you do it instead, using an iterator over the keys set or the equivalent Java8+ functional API:

Map<String, Object> bag = new LinkedHashMap<>();
bag.put("Foo", 1);
bag.put("Bar", "Hooray");

// Throws ConcurrentModificationException
for (Map.Entry<String, Object> e : bag.entrySet()) {
    if (e.getKey().equals("Foo")) {
        bag.remove(e.getKey());
    }
}

// Since Java 8
bag.keySet().removeIf(key -> key.equals("Foo"));

// Until Java 7
Iterator<String> it = bag.keySet().iterator();
while (it.hasNext()) {
    if (it.next().equals("Bar")) {
        it.remove();
    }
}

Can I use keySet to modify HashMap during iteration?

3 Answers3