How to replace HashMap Values while iterating over them in Java

Question

I am using a Runnable to automatically subtract 20 from a players cooldown every second, but I have no idea how to replace the value of a value as I iterate through it. How can I have it update the value of each key?

public class CoolDownTimer implements Runnable {
    @Override
    public void run() {
        for (Long l : playerCooldowns.values()) {
            l = l - 20;
            playerCooldowns.put(Key???, l);
        }
    }

}

Solution 1

Using Java 8:

map.replaceAll((k, v) -> v - 20);

---

Using Java 7 or older:

You can iterate over the entries and update the values as follows:

for (Map.Entry<Key, Long> entry : playerCooldowns.entrySet()) {
    entry.setValue(entry.getValue() - 20);
}

Solution 2

Well, you can't do it by iterating over the set of values in the Map (as you are doing now), because if you do that then you have no reference to the keys, and if you have no reference to the keys, then you can't update the entries in the map, because you have no way of finding out which key was associated with the value you just updated.

When working with Maps, you have two options for updates like this, iterate through each Map.Entry<K,V> in the Map, or you can iterate through the key Set. There are methods on Map to do both of these things. Personally, I would iterate through each Map.Entry<K,V>.

for (Map.Entry<String, Long> entry : playerCooldowns.entrySet()) {
    entry.setValue(entry.getValue() - 20);
}

Solution 3

Why not iterate over the Map.Entry objects ? Each Entry will give you the key and value and you don't have to perform an additional get() on the Map to get a value.

Comments

[Zach Sugano]

I can't believe I didn't think of that. Thanks.

[Tim Büthe]

Actually there is a FindBugs-Warning for that pattern: Inefficient use of keySet iterator instead of entrySet iterator And even if it doesn't matter in most cases, performance wise, it can make huge difference in some cases. Besides, I even think the code is clear and readable in both cases. Why use and post a pattern that clearly has some disadvantages and no advantage whatsoever? (Even if the disadvantages are arguably small)

[Tim Büthe]

@aioobe: I can imagine that this has different effects on different map implementation, sure, but is there one where the former form of iterating is more efficient then the latter? And therefore the FindBugs-Warning would be a false-positive? But you're right, we don't have to discuss that here. You answer is still okay, I guess.

[Alex Li]

Can you explain why this works? As I recall, you are not allowed to change elements of a collection while you iterator over them using a for each loop.

[aioobe]

You're not allowed to do structural changes such as adding or removing elements.

[Jochem Kuijpers]

@AlexLi The values of a hashmap do not influence the structure. You cannot change the keys of the hashmap, but you can change the values. Inserting and removing items is not allowed, thus you cannot change the key of an entry. The iterator often allows you to remove items though, because it has knowledge of the underlying structure and can prevent bad things from happening (skipping items, double-iterating items, iterating over random garbage, etc.).

[Kasper Nielsen]

As mentioned above I believe the keySet loop in the java 7 section to be wrong. As specified in docs.oracle.com/javase/8/docs/api/java/util/Map.html#keySet-‌​- the behavior of the iteration is undefined if the map is changed - which it could be using put, even if the key remains constant. The entry set example does not have the same flaw, as that is modifying the value directly, which is allowed by docs.oracle.com/javase/8/docs/api/java/util/Map.html#entrySe‌​t--

[aioobe]

@KasperNielsen good point. If it's the same restriction as for lists this refers to structural modifications (addition / removal of entries). Regardless, I've updated the answer to be on the safe side here. The entrySet is probably faster anyway. Thanks!