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.
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
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.
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.
