On 04/28/2014 10:20 AM, Paul Sandoz wrote:
On Apr 25, 2014, at 7:31 PM, Peter Levart <[email protected]> wrote:
Hi Paul,
To make any sense of null return from the mergeFunction, which could mean that
an entry with that key is absent from resulting map, such Collector would have
to keep some more state - the collecting map (which is also returned at the
end) and the set of removed keys. For example:
public static <T, K, U, M extends Map<K, U>>
Collector<T, ?, M> toMap(Function<? super T, ? extends K> keyMapper,
Function<? super T, ? extends U> valueMapper,
BinaryOperator<U> mergeFunction,
Supplier<M> mapSupplier) {
class State {
final M map = mapSupplier.get();
final Set<K> removedKeys = new HashSet<>();
M map() { return map; }
}
BiConsumer<State, T> accumulator = (state, element) -> {
K k = keyMapper.apply(element);
if (!state.removedKeys.contains(k)) {
U u = state.map.merge(k, valueMapper.apply(element),
mergeFunction);
if (u == null) state.removedKeys.add(k);
}
};
BinaryOperator<State> merger = (state1, state2) -> {
state1.map.keySet().removeAll(state2.removedKeys);
state2.map.keySet().removeAll(state1.removedKeys);
state1.removedKeys.addAll(state2.removedKeys);
for (Map.Entry<K,U> e : state2.map.entrySet()) {
U u = state1.map.merge(e.getKey(), e.getValue(),
mergeFunction);
if (u == null) state1.removedKeys.add(e.getKey());
}
return state1;
};
return new CollectorImpl<>(State::new, accumulator, merger, State::map, CH_ID);
}
...but the concurrent variant would be tricky to implement.
Yes, i suspect a finishing stage would be required to clean up the map.
The simplest thing to do is throw an NPE if Map.merge returns null. It would be
rather surprising for partial map value merging to have a global subtractive
effect (esp. if that could occur non-determinisitically when encounter order of
processing values is not important).
Collectors.toMap and groupingBy are already null-hostile, Map.merge (and your
update using putIfAbsent preserving the same merge behaviour) will throw an NPE
on a null input value.
Plus I now realize that throwing an NPE after removal has occurred is OK since
that side-effect should never be observed as resulting map will not be returned
to the caller.
You could use the null return from Map.merge() as a signal to throw NPE,
but this is only 100% safe in to*Map() methods that don't take a
mapSupplier.
Collections.toConcurrentMap(...., Supplier<M> mapSupplier) could be
abused by someone knowing that the supplier is called exactly once and
that the returned map is the same instance returned from the supplier.
For example:
ConcurrentMap<Integer, String> map = new ConcurrentHashMap<>();
Stream<Integer> s1 = Stream.of(1, 2, 3);
Stream<Integer> s2 = Stream.of(4, 5, 6);
s1.collect(toConcurrentMap(e -> e, e -> "" + e, (v1, v2) -> v1,
() -> map));
s2.collect(toConcurrentMap(e -> e, e -> "" + e, (v1, v2) -> v1,
() -> map));
System.out.println(map);
...in that case a null return from mergeFunction could be observed in
the result.
Regards, Peter
Paul.
