How to remove multiple elements from Set/Map AND knowing which ones were removed?

Question

I have a method that has to remove any element listed in a (small) Set<K> keysToRemove from some (potentially large) Map<K,V> from. But removeAll() doesn't do, as I need to return all keys that were actually removed, since the map might or might not contain keys that require removal.

Old-school code is straight forward:

public Set<K> removeEntries(Map<K, V> from) {
    Set<K> fromKeys = from.keySet();
    Set<K> removedKeys = new HashSet<>();
    for (K keyToRemove : keysToRemove) {
        if (fromKeys.contains(keyToRemove)) {
            fromKeys.remove(keyToRemove);
            removedKeys.add(keyToRemove);
        }
    }
    return removedKeys;
}

The same, written using streams:

Set<K> fromKeys = from.keySet();
return keysToRemove.stream()
        .filter(fromKeys::contains)
        .map(k -> {
            fromKeys.remove(k);
            return k;
        })
        .collect(Collectors.toSet());

I find that a bit more concise, but I also find that lambda too clunky.

Any suggestions how to achieve the same result in less clumsy ways?

Answer 1

The “old-school code” should rather be

public Set<K> removeEntries(Map<K, ?> from) {
    Set<K> fromKeys = from.keySet(), removedKeys = new HashSet<>(keysToRemove);
    removedKeys.retainAll(fromKeys);
    fromKeys.removeAll(removedKeys);
    return removedKeys;
}

Since you said that keysToRemove is rather small, the copying overhead likely doesn’t matter. Otherwise, use the loop, but don’t do the hash lookup twice:

public Set<K> removeEntries(Map<K, ?> from) {
    Set<K> fromKeys = from.keySet();
    Set<K> removedKeys = new HashSet<>();
    for(K keyToRemove : keysToRemove)
        if(fromKeys.remove(keyToRemove)) removedKeys.add(keyToRemove);
    return removedKeys;
}

You can express the same logic as a stream as

public Set<K> removeEntries(Map<K, ?> from) {
    return keysToRemove.stream()
        .filter(from.keySet()::remove)
        .collect(Collectors.toSet());
}

but since this is a stateful filter, it is highly discouraged. A cleaner variant would be

public Set<K> removeEntries(Map<K, ?> from) {
    Set<K> result = keysToRemove.stream()
        .filter(from.keySet()::contains)
        .collect(Collectors.toSet());
    from.keySet().removeAll(result);
    return result;
}

and if you want to maximize the “streamy” usage, you can replace from.keySet().removeAll(result); with from.keySet().removeIf(result::contains), which is quiet expensive, as it is iterating over the larger map, or with result.forEach(from.keySet()::remove), which doesn’t have that disadvantage, but still, isn’t more readable than removeAll.

All in all, the “old-school code” is much better than that.

Answer 2

More concise solution, but still with unwanted side effect in the filter call:

Set<K> removedKeys =
    keysToRemove.stream()
                .filter(fromKeys::remove)
                .collect(Collectors.toSet());

Set.remove already returns true if the set contained the specified element.

 P.S. In the end, I would probably stick with the "old-school code".

Answer 3

I wouldn’t use Streams for this. I would take advantage of retainAll:

public Set<K> removeEntries(Map<K, V> from) {
    Set<K> matchingKeys = new HashSet<>(from.keySet());
    matchingKeys.retainAll(keysToRemove);

    from.keySet().removeAll(matchingKeys);

    return matchingKeys;
}

Answer 4

You can use the stream and the removeAll

Set<K> fromKeys = from.keySet();
Set<K> removedKeys = keysToRemove.stream()
    .filter(fromKeys::contains)
    .collect(Collectors.toSet());
fromKeys.removeAll(removedKeys);
return removedKeys;