j3d
j3d

Reputation: 9724

Java Stream: How to Sum Value If Element Already Exists

Given the following key...

private class PositionKey {
    String assetId;
    String currencyId;
}

... I need to create a Map<PositionKey, BigDecimal>, where if a given key does not exist, then a new entry shall be added, otherwise, if a given key already exists, then its values shall be increased. Here below is my attempt:

public Map<PositionKey, BigDecimal> getMap(final Long portfolioId) {

    final Map<PositionKey, BigDecimal> map = new HashMap<>();

    // getPositions() returns all the positions associated with the given portfolio
    return getPositions(portfolioId).stream()
        .collect(
            toMap(
                position ->
                    new PositionKey(
                        position.getAssetId(),
                        position.geCurrencyId()),
                position -> position.geValue(),
                (newValue, oldValue) -> oldValue != null ? oldValue.add(newValue) : newValue,
                () -> map));
}

What I dislike in my implementation above, is the fact that I create the Map before streaming the positions. Is there a better way to implement this? Thank you very much.

Upvotes: 0

Views: 489

Answers (2)

Alexander Ivanchenko
Alexander Ivanchenko

Reputation: 28978

In case if you have no specific requirements regarding the Map implementation, there's no need to use a version of Collector toMap() which expects a mapFactory.

Instead, use three-args version, which expects only functions mapping keys/values and resolving duplicates, namely: keyMapper, valueMapper and a mergeFunction toMap(keyMapper, valueMapper, mergeFunction).

Also, you have messed with the order of arguments in the mergeFunction. The previously associated Value comes first, i.e. oldValue then newValue (not the opposite, although in this case it has no impact on the result since mergeFunction only performs addition of BigDecimal values). And as @Holger has pointed out in the comments, there's no need to perform null-check unless valueMapper function can produce null.

(oldValue, newValue) -> oldValue.add(newValue)

Which can be translated into a Method reference:

BigDecimal::add

That's how collect() operation might look like in this case:

.collect(Collectors.toMap(
    position -> new PositionKey(
        position.getAssetId(),
        position.geCurrencyId()
    ),
    Position::geValue,
    BigDecimal::add
))

And you would be provided with the default general purpose implementation of the Map interface by default, which is for now HashMap (if there would be another general purpose implementation which come as a recommended replacement of HashMap you'll get it for free without changing the code). Hence, providing HashMap::new as a mapFactory is frankly pointless.

A use case for the four-args version of toMap() expecting a mapFactory would be a situation when you need a special purpose Map implementation, like EnumMap, TreeMap, LinkedHashMap, etc.

Upvotes: 1

Mostafa Nazari
Mostafa Nazari

Reputation: 716

simple! replace () -> map with either () -> new HashMap<>() or HashMap::new

Upvotes: 2

Related Questions