Mfcttrf

I have a method that has to remove all elements found 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.

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?

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.

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".

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;

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;

You can use this:

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

It's similar to Oleksandr's answer, but avoiding the side effect. But I would stick with that answer, if you are looking for performance.

Alternatively you could use Stream.peek() for the remove, but be careful with other side effects (see the comments). So I would not recommend that.

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