Reputation: 16428
I've got a map containing some keys (Strings) and values (POJOs)
I want to iterate through this map and alter some of the data in the POJO.
The current code I've inherited removes the given entry, and adds it back in after making some changes to the POJO.
This doesn't work well, since you shouldn't be modifying a map whilst your iterating through it (method is synchronised, but ConcurrentModificationException still appears)
My question is, if I need to iterate over a map and change values, what are the best practices/methods I can use for doing so? To create a separate map and build that up as I go, then return the copy?
Upvotes: 23
Views: 58839
Reputation: 299048
For such purposes you should use the collection views a map exposes:
Example: convert the values of all keys that contain an upperscore to uppercase
for(Map.Entry<String, String> entry:map.entrySet()){
if(entry.getKey().contains("_"))
entry.setValue(entry.getValue().toUpperCase());
}
Actually, if you just want to edit the value objects, do it using the values collection. I assume your map is of type <String, Object>
:
for(Object o: map.values()){
if(o instanceof MyBean){
((Mybean)o).doStuff();
}
}
Upvotes: 8
Reputation: 719229
Iterating over a Map
and adding entries at the same time will result in a ConcurrentModificationException
for most Map
classes. And for the Map
classes that don't (e.g. ConcurrentHashMap
) there is no guarantee that an iteration will visit all entries.
Depending on exactly what it is you are doing, you may be able to do the following while iterating:
Iterator.remove()
method to remove the current entry, orMap.Entry.setValue()
method to modify the current entry's value.For other types of change, you may need to:
Map
from the entries in the current Map
, or Map
.And finally, the Google Collections and Apache Commons Collections libraries have utility classes for "transforming" maps.
Upvotes: 18
Reputation: 1074959
Two options:
The current code I've inherited removes the given entry, and adds it back in after making some changes to the POJO.
Are you changing the reference to the POJO? E.g., so the entry points to something else entirely? Because if not, there's no need to remove it from the map at all, you can just change it.
If you do need to actually change the reference to the POJO (e.g., the value of the entry), you can still do that in place by iterating over the Map.Entry
instances from entrySet()
. You can use setValue
on the entry, which doesn't modify what you're iterating over.
Example:
Map<String,String> map;
Map.Entry<String,String> entry;
Iterator<Map.Entry<String,String>> it;
// Create the map
map = new HashMap<String,String>();
map.put("one", "uno");
map.put("two", "due");
map.put("three", "tre");
// Iterate through the entries, changing one of them
it = map.entrySet().iterator();
while (it.hasNext())
{
entry = it.next();
System.out.println("Visiting " + entry.getKey());
if (entry.getKey().equals("two"))
{
System.out.println("Modifying it");
entry.setValue("DUE");
}
}
// Show the result
it = map.entrySet().iterator();
while (it.hasNext())
{
entry = it.next();
System.out.println(entry.getKey() + "=" + entry.getValue());
}
The output (in no particular order) is:
Visiting two
Modifying it
Visiting one
Visiting three
two=DUE
one=uno
three=tre
...without any modification exception. You will probably want to synchronize this in case something else is also looking at / mucking with that entry.
Upvotes: 24
Reputation: 9150
In order to provide a proper answer, you should explain a bit more, what you are trying to achieve.
Still, some (possibly useful) advice:
Which approach is best depends heavily on your application, it is difficult to give you any "best practice". As always, make your own benchmark with realistic data.
Upvotes: 2
Reputation: 15259
Another approach, somewhat tortured, is to use java.util.concurrent.atomic.AtomicReference
as your map's value type. In your case, that would mean declaring your map of type
Map<String, AtomicReference<POJO>>
You certainly don't need the atomic nature of the reference, but it's a cheap way to make the value slots rebindable without having to replace the entire Map.Entry
via Map#put()
.
Still, having read some of the other responses here, I too recommend use of Map.Entry#setValue()
, which I had never needed nor noticed until today.
Upvotes: 0
Reputation: 89179
Try using ConcurrentHashMap.
From JavaDoc,
A hash table supporting full concurrency of retrievals and adjustable expected concurrency for updates.
For ConcurrentModificationException to occur, generally:
it is not generally permissible for one thread to modify a Collection while another thread is iterating over it.
Upvotes: 0
Reputation: 26713
Create a new map (mapNew). Then iterate over the existing map (mapOld), and add all changed and transformed entries into mapNew. After the iteration is complete, put all values from mapNew to mapOld. This might not be good enough if the amount of data is large though.
Or just use Google collections - they have Maps.transformValues()
and Maps.transformEntries()
.
Upvotes: 3