Avoiding map.get(key) method

Question

I have the following code but i saw that retrieving values from a Map while iterating over the Map keys with keySet() is a mistake even with findBugs i get the warning WMI_WRONG_MAP_ITERATOR

for(String elementId : mapElements.keySet()){

     element = mapElements.get(elementId); 

     doSomething(element);
}

so why exactly is this not good and how can i fix it ?

Thanks.

Enthrone

This is of course explained in the documentation (findbugs.sourceforge.net/…) – Renaterenato

Answer 1

If you're iterating over everything in a map, you might as well do:

for (Map.Entry<String, String> entry : mapElements.entrySet()) {
    String key = entry.getKey();
    String value = entry.getValue();
    // Use the key and the value
}

Or if you don't really need the key, just iterate over the values:

for (String value : mapElements.values()) {
    doSomething(value);
}

EDIT: syntax

Mullin

Answer 2

Retrieving values from a map while iterating over the map itself is not an issue - what becomes an issue is when you are modiying the map while simultaneously iterating over it. In your case , this doesnt seem to be the case , so this is itself not dangerous.

When you iterate over a map , the iterator you get is based on a snapshot of all map entries at the time you obtain the iterator. Upon subsequent midification , this iterator's behaviour becomes undefined. This is what is not good. But again, in your case this does not apply because you are not updating the map.

Ermentrude

Answer 3

Another point is that looking up the value of each key may be expensive if the map is large. So Jon Skeet's suggestion is more efficient. However, I admit the code for iterating over a map's entry set is a bit clumsy.

Exegesis