SergVasiliev 1 июл в 05:21

.NET: итерируемую в foreach коллекцию изменять нельзя. Или…

Средний

4 мин

9.3K

Программирование*.NET*C#*

+37

Комментарии 27

KamushekDev 1 июл в 06:21

Осталось понять зачем они сделали это изменение в Dictionary

mayorovp 1 июл в 08:52

Скорее всего - потому что могли. Просто в какой-то момент пришли к такой структуре Dictionary, в которой вызов Remove посреди итерации ничего не ломает.

freeExec 1 июл в 17:03

А чего он ломает в списках?

troepolik 1 июл в 21:37

При удалении все последующие элементы смещаются и дальше в зависимости от места удаления относительно текущей позиции итерации- ты можешь один и тот же элемент проитерировать 2 раза или пропустить один из элементов.

mayorovp 1 июл в 23:05

Проитерировать два раза - это при добавлении, при удалении может случиться только пропуск.

CrazyElf 1 июл в 08:44

Если очень нужно лучше всегда делать копию коллекции и итерироваться по ней уже. Но так то ещё лучше вообще без такого сценария обойтись, а сделать вычитание множеств или ещё что-то более высокоуровневое, чем удаление отдельных элементов по одному.

ritorichesky_echpochmak 1 июл в 10:24

сделать вычитание множеств или ещё что-то более высокоуровневое

А можно пару примеров на пальцах? Кажется, в плане потребления ресурсов это не будет "ещё лучше"

CrazyElf 1 июл в 11:48

Зависит от конкретного use case. Где-то вообще будет лучше фильтрация коллекции в новую через Linq. Нужно смотреть - сколько вообще данных, какой процент предполагается удалить, и т.п.

mynameco 1 июл в 19:27

У нас в юнити борьба даже за боксинг итераторов. А тут целую копию коллекции делать. Сейчас выкручиваемся таким ужасным способом, что стыдно рассказывать. После удаления, делаем goto перед циклом, и ищем следующий элемент. При том количестве элементов и удаляемых данных, все всех устраивает.

VanKrock 2 июл в 19:30

Звучит интересно, а можете подсказать какая у вас исходная коллекция? List?

aamonster 1 июл в 09:19

Чувак, попиши на крестиках – там нарушение подобных контрактов не бросит тебе простое и понятное исключение, а приведёт к UB. И ты навсегда перестанешь полагаться на исключения в таких местах, и начнёшь использовать filter или ещё что.

Кстати, емнип JavaScript позволяет менять коллекции во время итерации. Но я настолько привык так не делать...

ЗЫ: В опросе не хватает варианта "мне всё равно".

domix32 1 июл в 13:13

JS в худшем случае просто бросит исключение про undefined т.к. его задача не ронять фронт никогда.

использовать filter или ещё что

во-во, удивляет желание людей заниматься подобным в обычных циклах.

aamonster 1 июл в 13:59

JS – это не только фронт, но и на фронте не всегда лучше "не ронять" – порой лучше упасть и дать юзеру перезагрузить страницу, чем, к примеру, испортить его данные.
Но суть в том, что там изменение объекта во время итерации (ну, одного из видов объектов) не просто "в худшем случае бросит исключение", а абсолютно законно и ведёт себя предсказуемо: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...in#deleted_added_or_modified_properties
Но всё равно проще так не делать, чтобы не выстрелить себе в ногу при переходе с языка на язык и чтобы тот, кто читает твой код, не должен был лезть в документацию.

Понятно, почему так сделано в JS, понятно, почему так не сделано в C++, ну и подход .net тоже понятен (раз уж можно задёшево предупредить юзера, что он стреляет себе в ногу – почему нет?) и даже понятно, почему в текущей версии нет exception (раз уж мы не ломаем коллекцию при итерации по ней – в этом нет большого смысла).

SergVasiliev 1 июл в 13:51

попиши на крестиках – там нарушение подобных контрактов не бросит тебе простое и понятное исключение, а приведёт к UB

При чём тут плюсы вообще? В C++ одно поведение, в C# — другое. Речь вообще ни разу не шла о "полагаться на исключения". Очень сомневаюсь, что в C# / .NET кто-то пытается менять итерируемую коллекцию с расчётом на исключения — "контракт" о том, что так в принципе не надо делать.

ЗЫ: В опросе не хватает варианта "мне всё равно".

Если всё равно, можно не голосовать или воздержаться.

aamonster 1 июл в 14:01

Так контракт вам никто не мешает соблюдать (лично я его соблюдаю даже в JS, потому как не желаю запоминать, где он есть, а где его нет: мой код должен читаться без привлечения документации). Просто больше никто не будет проверять, что вы его соблюдаете, потому что его нарушение перестало ломать данные.
И да, C++ я привёл в качестве примера, поскольку там был ровно тот же контракт. Просто никто не следил за его выполнением, а нарушение могло сломать вам вообще всё и без всякой диагностики.

aegoroff 1 июл в 09:25

Могу предположить, что сделали это лишь для удобства, несколько странно конечно, но как есть. К примеру надо удалить из HashSet, существующие элементы по какому-либо условию, - к примеру (если хэшсет чисел), удалить все четные, которые там есть, если это строки, удалить строки удовлетворяющие каким-либо условиям. Использовать для удаления всего - глупо ибо есть метод Clear, использовать для удаления заранее известных элементов - тоже глупо, ибо итерироваться можно по этому, известному множеству.

А вот в случае что я упомянул, иначе бы пришлось сначала найти все такие элементы, скопировав их в другую коллекцию (привет выделение памяти и дополнительный проход по элементам), и только потом, итерируясь по этой новой коллекции, поменять HashSet

a-tk 1 июл в 11:11

Очистка от элементов по какому-то критерию во время итерирования - вполне себе юз-кейс.

ignorance 1 июл в 13:59

Есть встроенная Hashset.RemoveWhere(Predicate)

aegoroff 1 июл в 14:41

public int RemoveWhere(Predicate<T> match)
{
    if (match == null)
    {
        ThrowHelper.ThrowArgumentNullException(ExceptionArgument.match);
    }

    Entry[]? entries = _entries;
    int numRemoved = 0;
    for (int i = 0; i < _count; i++)
    {
        ref Entry entry = ref entries![i];
        if (entry.Next >= -1)
        {
            // Cache value in case delegate removes it
            T value = entry.Value;
            if (match(value))
            {
                // Check again that remove actually removed it.
                if (Remove(value))
                {
                    numRemoved++;
                }
            }
        }
    }

    return numRemoved;
}

так она по сути и использует то, что описано в статье

ignorance 1 июл в 16:19

Только она никак не проверяет инкремент версий, соответственно, изменение поведения Remove на ней не должно было отразиться

Hallfire 1 июл в 19:21

так она по сути и использует то, что описано в статье

насколько я понимаю, нет
эта функция использует цикл for для прохода по внутреннему массиву (при этом, судя по этой реализации метода Remove из внутреннего массива ничего не удаляется, а меняется значение на default(T))
но не итератор

aegoroff 1 июл в 19:33

Не совсем так - RemoveWhere идет по списку значений, а Remove по списку бакетов, а это не совсем одно и тоже, точнее совсем разное - в один бакет может попасть несколько разных значений (если у них хэш код один), а в списке значений такое невозможно. Но собственно, ни один, ни другой метод не меняют поле _version у коллекции, изменение которого, приводит к инвалидации итератора.

Hallfire 1 июл в 20:01

а это не совсем одно и тоже, точнее совсем разное

да, но в данном случае это не важно, т.к. речь про способ прохода по коллекции

и то что методы не меняют поле _version будет играть роль только если использовать Remove или RemoveWhere внутри foreach (или если самостоятельно использовать итератор)

зы

так она по сути и использует то, что описано в статье

я трактовал это сообщение не как

метод RemoveWhere тоже использует внутри себя Remove

но как

метод RemoveWhere "пользуется" описанным в статье "багом", поэтому успешно выполняется и не падает при вызове Remove

соотв. в предыдущем комментарии я и хотел сказать, что это не так, т.к. даже если бы в Remove менялось поле _version, то метод RemoveWhere всё так-же нормально работал бы, из-за того, что в нём не используется итератор, а значит и выкидывать исключение некому

casualkex 3 июл в 08:28

Возможно позволяют такое делать потому что элементы списка имеют порядок, а элементы словаря/сета не обязаны иметь порядка

VCoder 4 июл в 12:24

Интересное исследование - спасибо, что поделились!

В том коммите есть тесты

        protected override ModifyOperation ModifyEnumeratorThrows => PlatformDetection.IsNetFramework ? base.ModifyEnumeratorThrows : (base.ModifyEnumeratorAllowed & ~ModifyOperation.Remove);

        protected override ModifyOperation ModifyEnumeratorAllowed => PlatformDetection.IsNetFramework ? base.ModifyEnumeratorAllowed : ModifyOperation.Overwrite | ModifyOperation.Remove;

Я покопался в исходниках - думаю, что разница в поведении заключается в следующем:

List хранит данные в массиве. Итератор держит индекс текущего элемента. При изменении коллекции (Add/Remove) индекс становится не действительным.
HashSet (как и Dictionary) хранят данные в бакетах (bucket), номер бакета определяется вызовом GetHashCode. Данные в бакетах хранятся в связном списке (насколько я понимаю linked list). Соответственно при удалении элемента меняются ссылки в списке - при этом связный список остается валидным. Технически, при добавлении элемента связный список тоже остается валидным, но есть нюанс ;) - добавленный элемент может оказаться в бакете который мы уже посетили или в бакете который нам еще предстоит посетить. Это неопределенное поведение (за который часто ругают плюсы) поэтому при добавлении элемента энумератор кидает исключение.

Я бы поостерегался закладываться на такое поведение - оно может поменяться в новых версиях.

posledam 4 июл в 23:19

Что-то удалять из List<T> плохая затея, так как удаление элементов из списка это дорого. Итерирование же, совместно с изменением -- чисто логически, сама по себе, не адекватная операция. Коллекции, каждая, имеет свою специфику + есть ещё особенности реализации. Да и зачем вообще такое делать?

SergVasiliev 5 июл в 00:33

Что-то удалять из List плохая затея

List<T> — для примера. Как я писал:

Мы рассмотрим пример с List, так как он чуть проще.

Коллекции, каждая, имеет свою специфику + есть ещё особенности реализации.

Речь о том, что для всех коллекций, независимо от специфики и особенностей реализации был контракт: изменяешь во время обхода через итератор? Получай исключение. Теперь — с нюансами.

Да и зачем вообще такое делать?

Например, если хочется отфильтровать коллекцию:

foreach (var item in collection)
{
  ...
  if (condition)
  {
    collection.Remove(item);
  }
}

Понятно, что так делать нельзя (теперь иногда можно*), крутые ребята так не пишут и бла-бла-бла, но речь про интеншн.

Зарегистрируйтесь на Хабре, чтобы оставить комментарий

.NET: итерируемую в foreach коллекцию изменять нельзя. Или…

Комментарии 27

Публикации

Истории