Как удалить несколько элементов из набора/карты и узнать, какие из них были удалены?
У меня есть метод, который должен удалить любой элемент, указанный в (маленьком) Set<K> keysToRemove, из некоторого (потенциально большого) Map<K,V> from. Но removeAll() не подходит, так как мне нужно вернуть все ключи, которые были фактически удалены, поскольку карта может содержать или не содержать ключи, требующие удаления.
Код старой школы прямолинеен:
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;
}
То же самое, написанное с использованием потоков:
Set<K> fromKeys = from.keySet();
return keysToRemove.stream()
.filter(fromKeys::contains)
.map(k -> {
fromKeys.remove(k);
return k;
})
.collect(Collectors.toSet());
Я нахожу это немного более кратким, но я также нахожу эту лямбду слишком неуклюжей.
Любые предложения, как добиться того же результата менее неуклюжими способами?
Я считаю и делаю вывод из ответов здесь, улучшение, которое происходит в основном из любых изменений, заключается в использовании if (fromKeys.remove(keyToRemove)) { removedKeys.add(keyToRemove); } вместо использования как содержимого, так и удаления в if (fromKeys.contains(keyToRemove)) { fromKeys.remove(keyToRemove); removedKeys.add(keyToRemove); }
Ответы 6
Более краткое решение, но все еще с нежелательным побочный эффект в вызове filter:
Set<K> removedKeys =
keysToRemove.stream()
.filter(fromKeys::remove)
.collect(Collectors.toSet());
Set.remove уже возвращает true, если set содержит указанный элемент.
P.S. В конце концов, я бы, вероятно, придерживался «кода старой школы».
Точно моя мысль ;) - Это просто кажется немного хакерским, потому что мы «фильтруем» метод, который на самом деле представляет собой побочный эффект.
Вы можете использовать поток и removeAll
Set<K> fromKeys = from.keySet();
Set<K> removedKeys = keysToRemove.stream()
.filter(fromKeys::contains)
.collect(Collectors.toSet());
fromKeys.removeAll(removedKeys);
return removedKeys;
Вы можете использовать это:
Set<K> removedKeys = keysToRemove.stream()
.filter(from::containsKey)
.collect(Collectors.toSet());
removedKeys.forEach(from::remove);
Это похоже на ответ Александр, но без побочного эффекта. Но я бы придерживался этого ответа, если вам нужна производительность.
В качестве альтернативы вы можете использовать Stream.peek() для удаления, но будьте осторожны с другими побочными эффектами (см. комментарии). Так что я бы не рекомендовал это.
Set<K> removedKeys = keysToRemove.stream()
.filter(from::containsKey)
.peek(from::remove)
.collect(Collectors.toSet());
Никогда не используйте peek ни для чего, кроме отладки! См. stackoverflow.com/questions/47356992/… и связанные с ним вопросы.
@MichaelA.Schaffrath Имеет смысл. Забавный факт: я снова попробовал свою первоначальную лямбду с картой. IntelliJ предлагает заменить этот вызов map() на peek() ;-)
Или более общее: В потоках Java просмотр действительно только для отладки?
Я бы не стал использовать потоки для этого. Я бы воспользовался сохранить все:
public Set<K> removeEntries(Map<K, V> from) {
Set<K> matchingKeys = new HashSet<>(from.keySet());
matchingKeys.retainAll(keysToRemove);
from.keySet().removeAll(matchingKeys);
return matchingKeys;
}
Это указывает в правильном направлении, но вы копируете набор ключей «потенциально большого» from карты, в то время как вместо этого вы можете скопировать «маленький» keysToRemove, поскольку пересечение a и b такое же, как у b и a. Кроме того, matchingKeys потенциально меньше, чем keysToRemove, поэтому removeAll(matchingKeys) предпочтительнее.
@Holger Я понимаю вашу точку зрения, но Набор просто копирует ссылки, что мне кажется безобидным, если только размер Карты действительно не огромен. Вы правы насчет removeAll(matchingKeys). Обновлено.
Это не просто копирование ссылок, а хеширование. И поскольку в OP указаны ожидаемые размеры, а поменять их местами тривиально, я бы сделал это. На самом деле я сделал.
«Кодекс старой школы» скорее должен быть
public Set<K> removeEntries(Map<K, ?> from) {
Set<K> fromKeys = from.keySet(), removedKeys = new HashSet<>(keysToRemove);
removedKeys.retainAll(fromKeys);
fromKeys.removeAll(removedKeys);
return removedKeys;
}
Поскольку вы сказали, что keysToRemove довольно мал, накладные расходы на копирование, вероятно, не имеют значения. В противном случае используйте цикл, но не выполняйте поиск хеша дважды:
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;
}
Вы можете выразить ту же логику, что и поток, как
public Set<K> removeEntries(Map<K, ?> from) {
return keysToRemove.stream()
.filter(from.keySet()::remove)
.collect(Collectors.toSet());
}
но поскольку это фильтр с отслеживанием состояния, он крайне не рекомендуется. Более чистый вариант был бы
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;
}
и если вы хотите максимизировать «потоковое» использование, вы можете заменить from.keySet().removeAll(result); на from.keySet().removeIf(result::contains), что довольно дорого, так как перебирает большую карту, или на result.forEach(from.keySet()::remove), у которого нет этого недостатка, но все же t более читабелен, чем removeAll.
В общем, «код старой школы» намного лучше.
Работал с существующим кодом. Разве public Set<K> removeEntries(Map<K, ?> from) { Set<K> removedKeys = new HashSet<>(); for (K keyToRemove : keysToRemove) { if (from.keySet().remove(keyToRemove)) { removedKeys.add(keyToRemove); } } return removedKeys; } не будет лучшим старым школьным кодом, чем retainAll, removeAll решение. Я хотел сосредоточиться (придираться) к одной итерации, а не к двум итерациям с их реализацией. (Может быть, я просто ошибаюсь, делая такой вывод.)
@Naman это то, что я опубликовал как второй вариант, как раз на тот случай, когда итерация имеет значение. Однако комбинация retainAll/removeAll будет перебирать набор, который был указан OP как довольно маленький.
О да, второй вариант действительно такой же. Но не будет ли removeAll повторять fromKeys вместо keysToRemove? Что касается первого варианта потока, как насчет использования предиката для сокращения с помощью partitioningBy, написал образец кода?
@Naman касается деталей реализации, но я предполагаю, что он работает как AbstractSet.removeAll(…), если даже не наследует правильно этот метод: «Эта реализация определяет, что меньше из этого набора и указанной коллекции, вызывая метод size для каждого из них. …[так далее]». Использование предиката с состоянием для partitioningBy так же не рекомендуется, как и для filter, но с последним вы собираете еще один набор действительно нежелательных элементов…
Немного не по теме, но как бы вы проверили правильность вашего решения? Вы действительно пишете все с нуля в IntelliJ (public static void main и все) и проверяете? Спрашиваю, потому что я не знаком с потоками или тегом java.
@ cs95 ну да, для большинства ответов SO я пишу тестовый код либо с нуля, либо используя код вопроса в качестве отправной точки, если таковой имеется. В зависимости от контекста это может быть Netbeans, Eclipse или командная строка. Когда дело доходит до поведения, зависящего от компилятора, у меня также есть пакетные файлы для компиляции и запуска одного и того же исходного кода с разными JDK.
@ Наман, мое последнее предложение было написано в спешке. Я хотел сказать, что partitioningBy работает больше, когда нужно, когда нужен только один из двух наборов. Кроме того, это похоже на подход filter.
@Holger Я все равно понял, что вы имели в виду под другим набором. Спасибо ?
Если вы уже создаете минимальный воспроизводимый пример для своих тестов, почему бы не опубликовать его, чтобы другие могли быстрее его проверить? По крайней мере, я как правило пытаюсь это сделать вместе с некоторыми "фиктивными/тестовыми" данными и системными данными, показывающими результаты...
@ Marco13 Marco13 Я часто так делаю, особенно для вопросов, содержащих пример, но не каждый ответ будет полезен с примерами. Кроме того, не весь мой тестовый код является минимальным примером. А иногда он редактируется для других тестов, без одновременного включения всех тестов в код, поэтому перед публикацией требуется значительная очистка.
Чтобы добавить еще один вариант к подходам, можно также разбить ключи и вернуть требуемый Set как:
public Set<K> removeEntries(Map<K, ?> from) {
Map<Boolean, Set<K>> partitioned = keysToRemove.stream()
.collect(Collectors.partitioningBy(k -> from.keySet().remove(k),
Collectors.toSet()));
return partitioned.get(Boolean.TRUE);
}
Также оставляет возможность использовать ключи, которые не были частью набора ключей карты. (на всякий случай)
Как насчет того, чтобы просто собрать все ключи, которые можно удалить, а затем вызвать
removeAll()для этого отфильтрованного набора? Или как насчет «фильтрации» наfromKeys::remove?