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?

Answer 1

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));

Answer 2

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.

Answer 3

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();
    }
}