Refactoring if-else block to use Map.computeIfAbsent Map.computeIfPresent wrong result

Question

I have a class

public class Gezana {
    private String name;
    private long value;
}

and a list like:

List<Gezana> list = List.of(new Gezana("foo", 2), 
                            new Gezana("foo", 2),
                            new Gezana("foo", 2),
                            new Gezana("foo", 2), 
                            new Gezana("bar", 5),
                            new Gezana("bar", 5), 
                            new Gezana("bar", 5), 
                            new Gezana("doo", 9), 
                            new Gezana("doo", 9),
                            new Gezana("kee", 12));

I need to create a map using above list with name as key and sum of values as value. I used to do it in the following way:

Map<String, Long> oldMap = new HashMap<>(); 
for (Gezana gez : list) {            
    if (oldMap.containsKey(gez.getName())) {
        oldMap.put(gez.getName(), oldMap.get(gez.getName()) + gez.getValue());
    } else{
        oldMap.put(gez.getName(), gez.getValue());
    }          
}        
System.out.println("old map:" + oldMap);

I need to refactor the above and thought to use Map.computeIfAbsent & Map.computeIfPresent but getting not the expected result

Map<String, Long> map = new HashMap<>(); 
for (Gezana gez : list) {            
    map.computeIfAbsent(gez.getName(), k -> gez.getValue());     
    map.computeIfPresent(gez.getName(), (k, v)->  v + gez.getValue());              
}
System.out.println("new map:" + map);

Output:

old map:{bar=15, doo=18, foo=8, kee=12}
new map:{bar=20, doo=27, foo=10, kee=24}

It seems that the new map has one additional value for each key added, but i don't see the mistake in using the computeIfAbsent & computeIfPresent methods. Any hints?

Answer 1

The problem with this is that computeIfAbsent always adds to the map; computeIfPresent will then update the thing that computeIfAbsent maybe just added.

So, when a name is first encountered, its value is added twice - once by the computeIfAbsent, then by the computeIfPresent.

You could do it with computeIfAbsent/computeIfPresent like this:

map.computeIfAbsent(gez.getName(), k -> 0L);     
map.computeIfPresent(gez.getName(), (k, v)->  v + gez.getValue());

or compute:

map.compute(gez.getName(), (k, v) -> (v != null ? v : 0L) + get.getValue());

but it would be easier with merge:

oldMap.merge(gez.getName(), gez.getValue(), Long::sum);

Or, directly from the list:

Map<String, Long> map = 
    list.stream().collect(groupingBy(Gezana::getName, summingLong(Gezana::getValue)))