CODEWITHSUNDEEP
lambdajava-8collectors
Joseph
Joseph

Reputation: 172

Is mergeFunction of Collectors.toMap allowed to modify its arguments?

I have a stream of nested maps Stream<Map<String, Map<String, String>>> that I want to combine (using the outer key; assume that the inner keys are unique) by converting into a stream of entry sets and calling Collectors.toMap(...). In order to ensure that maps with duplicate outer keys are properly combined, I'm passing in the following BinaryOperator to toMap(...) function:

(existingMap, newMap) -> {
    existingMap.putAll(newMap);
    return existingMap;
}

The code seems to work for the time being, but I feel like I'm not using Collectors.toMap(...) as intended as I'm mutating the value inside the accumulator and the combiner.

Here's the full code snippet:

mapsToCombine.flatMap(map -> map.entrySet().stream()).collect(Collectors.toMap(Entry::getKey, Entry::getValue, (existingMap, newMap) -> {
    existingMap.putAll(newMap);
    return existingMap;
}));

Upvotes: 2

Views: 1393

Answers (2)

Holger
Holger

Reputation: 298539

You should be aware that the maps you are modifying are exactly the same maps contained in the source stream, so if your stream was built from a data structure (e.g. collection), this data structure will be modified in an unpredictable manner after the operation. It also implies that the whole operation can break, if the source contains the same map instance multiple times (which would be a case of violating the non-interference rule). Or if the source maps are immutable. Even worse, it may run multiple times without problems and suddenly break, perhaps not reproducible during debugging.

Generally, merging by modifying one of the inputs works fine, if this input is a result created during the stream operation, e.g. by the collector itself. You can achieve this easily by replacing the Entry::getValue function with e -> new HashMap<>(e.getValue()). Then, non-interference of the merge operation and mutability of the map is guaranteed, but it will create more temporary maps than not creating a map in the merge function saves.

Alternatively, you may use groupingBy which allows you to specify a collector for the values:

Map<String, Map<String, String>> result
  = mapsToCombine.flatMap(map -> map.entrySet().stream())
    .collect(Collectors.groupingBy(Entry::getKey, Collector.of(HashMap::new,
     (m,e) -> m.putAll(e.getValue()), (m1,m2) -> { m1.putAll(m2); return m1;})));

This will not modify any source map, but create only one mutable result map, so you can put into it when merging.

Upvotes: 3

Tagir Valeev
Tagir Valeev

Reputation: 100309

Seems that it's not explicitly specified, but with current implementation it's completely safe to do so.

Upvotes: 2

Related Questions