Редактирование значений словаря в цикле foreach

192

Я пытаюсь построить круговую диаграмму из словаря. Прежде чем отобразить круговую диаграмму, я хочу привести в порядок данные. Я удаляю любые кусочки пирога, которые будут составлять менее 5% от пирога, и помещаю их в «другой» кусок пирога. Однако я получаю Collection was modified; enumeration operation may not executeисключение во время выполнения.

Я понимаю, почему вы не можете добавлять или удалять элементы из словаря, просматривая их. Однако я не понимаю, почему вы не можете просто изменить значение для существующего ключа в цикле foreach.

Любые предложения по исправлению моего кода, будут оценены.

Dictionary<string, int> colStates = new Dictionary<string,int>();
// ...
// Some code to populate colStates dictionary
// ...

int OtherCount = 0;

foreach(string key in colStates.Keys)
{

    double  Percent = colStates[key] / TotalCount;

    if (Percent < 0.05)
    {
        OtherCount += colStates[key];
        colStates[key] = 0;
    }
}

colStates.Add("Other", OtherCount);
Aheho
источник

Ответы:

262

Установка значения в словаре обновляет его внутренний «номер версии», который делает недействительным итератор и любой итератор, связанный с коллекцией ключей или значений.

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

Обычный способ исправления такого рода вещей заключается в том, чтобы либо заранее скопировать коллекцию ключей и выполнить итерацию по копии, либо выполнить итерацию по исходной коллекции, но сохранить коллекцию изменений, которые вы примените после завершения итерации.

Например:

Сначала копируем ключи

List<string> keys = new List<string>(colStates.Keys);
foreach(string key in keys)
{
    double percent = colStates[key] / TotalCount;    
    if (percent < 0.05)
    {
        OtherCount += colStates[key];
        colStates[key] = 0;
    }
}

Или...

Создание списка модификаций

List<string> keysToNuke = new List<string>();
foreach(string key in colStates.Keys)
{
    double percent = colStates[key] / TotalCount;    
    if (percent < 0.05)
    {
        OtherCount += colStates[key];
        keysToNuke.Add(key);
    }
}
foreach (string key in keysToNuke)
{
    colStates[key] = 0;
}
Джон Скит
источник
24
Я знаю, что это старый, но если вы используете .NET 3.5 (или это 4.0?), Вы можете использовать и злоупотреблять LINQ следующим образом: foreach (строковый ключ в colStates.Keys.ToList ()) {...}
Machtyn
6
@Machtyn: Конечно, но вопрос был конкретно о .NET 2.0, иначе я бы наверняка использовал LINQ.
Джон Скит
Является ли «номер версии» частью видимого состояния словаря или детали реализации?
Аноним Трус
@SEinfringescopyright: он не виден напрямую; тот факт, что обновление словаря делает итератор недействительным , виден все же.
Джон Скит
Из Microsoft Docs .NET Framework 4.8 : оператор foreach является оболочкой для перечислителя, которая позволяет только читать из коллекции, но не записывать в нее. Поэтому я бы сказал, что это деталь реализации, которая может измениться в будущей версии. И что видно, так это то, что пользователь перечислителя нарушил свой контракт. Но я был бы неправ ... это действительно видно, когда словарь сериализован.
Аноним Трус
81

Позвоните ToList()в foreachпетле. Таким образом, нам не нужно копировать временную переменную. Это зависит от Linq, который доступен с .Net 3.5.

using System.Linq;

foreach(string key in colStates.Keys.ToList())
{
  double  Percent = colStates[key] / TotalCount;

    if (Percent < 0.05)
    {
        OtherCount += colStates[key];
        colStates[key] = 0;
    }
}
КОПАТЬ ЗЕМЛЮ
источник
Очень хорошее улучшение!
SpeziFish
1
Было бы лучше использовать, foreach(var pair in colStates.ToList())чтобы избежать доступа к ключу и значению, которое не требует вызова colStates[key]..
user2864740
21

Вы изменяете коллекцию в этой строке:

colStates [key] = 0;

Таким образом, вы по существу удаляете и повторно вставляете что-то в этот момент (в любом случае, что касается IEnumerable).

Если вы редактируете участника значения, которое вы храните, это будет нормально, но вы редактируете само значение, а IEnumberable это не нравится.

Решение, которое я использовал, состоит в том, чтобы исключить цикл foreach и просто использовать цикл for. Простой цикл for не проверяет изменения, которые, как вы знаете, не влияют на коллекцию.

Вот как вы можете это сделать:

List<string> keys = new List<string>(colStates.Keys);
for(int i = 0; i < keys.Count; i++)
{
    string key = keys[i];
    double  Percent = colStates[key] / TotalCount;
    if (Percent < 0.05)    
    {        
        OtherCount += colStates[key];
        colStates[key] = 0;    
    }
}
CodeFusionMobile
источник
Я получаю эту проблему, используя для цикла. словарь [индекс] [ключ] = "abc", но он возвращается к исходному значению "xyz"
Ник Чан Абдулла
1
Исправление в этом коде не является циклом for: оно копирует список ключей. (Это все равно будет работать, если вы преобразуете его в цикл foreach.) Решение с использованием цикла for будет означать использование colStates.Keysвместо keys.
idbrii
6

Вы не можете изменять ключи или значения непосредственно в ForEach, но вы можете изменять их члены. Например, это должно работать:

public class State {
    public int Value;
}

...

Dictionary<string, State> colStates = new Dictionary<string,State>();

int OtherCount = 0;
foreach(string key in colStates.Keys)
{
    double  Percent = colStates[key].Value / TotalCount;

    if (Percent < 0.05)
    {
        OtherCount += colStates[key].Value;
        colStates[key].Value = 0;
    }
}

colStates.Add("Other", new State { Value =  OtherCount } );
Джереми Фрей
источник
3

Как насчет того, чтобы просто выполнить несколько запросов linq к вашему словарю, а затем связать ваш график с результатами этих? ...

var under = colStates.Where(c => (decimal)c.Value / (decimal)totalCount < .05M);
var over = colStates.Where(c => (decimal)c.Value / (decimal)totalCount >= .05M);
var newColStates = over.Union(new Dictionary<string, int>() { { "Other", under.Sum(c => c.Value) } });

foreach (var item in newColStates)
{
    Console.WriteLine("{0}:{1}", item.Key, item.Value);
}
Скотт Айви
источник
Разве Linq доступен не только в версии 3.5? Я использую .net 2.0.
Ахехо
Вы можете использовать его с версии 2.0 со ссылкой на 3.5 версию System.Core.DLL - если вы не хотите этого делать, дайте мне знать, и я удалю этот ответ.
Скотт Айви
1
Я, вероятно, не пойду по этому пути, но, тем не менее, это хорошее предложение. Я предлагаю вам оставить ответ на месте, если кто-то с такой же проблемой наткнется на него.
Ахехо
3

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

Dictionary<string, int> collection = new Dictionary<string, int>();
collection.Add("value1", 9);
collection.Add("value2", 7);
collection.Add("value3", 5);
collection.Add("value4", 3);
collection.Add("value5", 1);

for (int i = collection.Keys.Count; i-- > 0; ) {
    if (collection.Values.ElementAt(i) < 5) {
        collection.Remove(collection.Keys.ElementAt(i)); ;
    }

}

Конечно, не идентичны, но вы можете быть заинтересованы в любом случае ...

Hugoware
источник
2

Вам нужно создать новый словарь из старого, а не модифицировать на месте. Что-то вроде (также перебираем KeyValuePair <,> вместо использования поиска ключа:

int otherCount = 0;
int totalCounts = colStates.Values.Sum();
var newDict = new Dictionary<string,int>();
foreach (var kv in colStates) {
  if (kv.Value/(double)totalCounts < 0.05) {
    otherCount += kv.Value;
  } else {
    newDict.Add(kv.Key, kv.Value);
  }
}
if (otherCount > 0) {
  newDict.Add("Other", otherCount);
}

colStates = newDict;
Ричард
источник
1

Начиная с .NET 4.5 Вы можете сделать это с ConcurrentDictionary :

using System.Collections.Concurrent;

var colStates = new ConcurrentDictionary<string,int>();
colStates["foo"] = 1;
colStates["bar"] = 2;
colStates["baz"] = 3;

int OtherCount = 0;
int TotalCount = 100;

foreach(string key in colStates.Keys)
{
    double Percent = (double)colStates[key] / TotalCount;

    if (Percent < 0.05)
    {
        OtherCount += colStates[key];
        colStates[key] = 0;
    }
}

colStates.TryAdd("Other", OtherCount);

Однако обратите внимание, что его производительность на самом деле намного хуже, чем у простого foreach dictionary.Kes.ToArray():

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

public class ConcurrentVsRegularDictionary
{
    private readonly Random _rand;
    private const int Count = 1_000;

    public ConcurrentVsRegularDictionary()
    {
        _rand = new Random();
    }

    [Benchmark]
    public void ConcurrentDictionary()
    {
        var dict = new ConcurrentDictionary<int, int>();
        Populate(dict);

        foreach (var key in dict.Keys)
        {
            dict[key] = _rand.Next();
        }
    }

    [Benchmark]
    public void Dictionary()
    {
        var dict = new Dictionary<int, int>();
        Populate(dict);

        foreach (var key in dict.Keys.ToArray())
        {
            dict[key] = _rand.Next();
        }
    }

    private void Populate(IDictionary<int, int> dictionary)
    {
        for (int i = 0; i < Count; i++)
        {
            dictionary[i] = 0;
        }
    }
}

public class Program
{
    public static void Main(string[] args)
    {
        BenchmarkRunner.Run<ConcurrentVsRegularDictionary>();
    }
}

Результат:

              Method |      Mean |     Error |    StdDev |
--------------------- |----------:|----------:|----------:|
 ConcurrentDictionary | 182.24 us | 3.1507 us | 2.7930 us |
           Dictionary |  47.01 us | 0.4824 us | 0.4512 us |
Охад Шнайдер
источник
1

Вы не можете изменить коллекцию, даже значения. Вы можете сохранить эти случаи и удалить их позже. Это закончилось бы так:

Dictionary<string, int> colStates = new Dictionary<string, int>();
// ...
// Some code to populate colStates dictionary
// ...

int OtherCount = 0;
List<string> notRelevantKeys = new List<string>();

foreach (string key in colStates.Keys)
{

    double Percent = colStates[key] / colStates.Count;

    if (Percent < 0.05)
    {
        OtherCount += colStates[key];
        notRelevantKeys.Add(key);
    }
}

foreach (string key in notRelevantKeys)
{
    colStates[key] = 0;
}

colStates.Add("Other", OtherCount);
Самуэль Каррихо
источник
Вы можете изменить коллекцию. Вы не можете продолжать использовать итератор для измененной коллекции.
user2864740
0

Отказ от ответственности: я не делаю много C #

Вы пытаетесь изменить объект DictionaryEntry, который хранится в HashTable. В Hashtable хранится только один объект - ваш экземпляр DictionaryEntry. Изменение ключа или значения достаточно, чтобы изменить HashTable и сделать перечислитель недействительным.

Вы можете сделать это вне цикла:

if(hashtable.Contains(key))
{
    hashtable[key] = value;
}

сначала создав список всех ключей значений, которые вы хотите изменить, и переберите этот список.

камбий
источник
0

Вы можете сделать копию списка dict.Values, затем вы можете использовать List.ForEachлямбда-функцию для итерации (или foreachцикл, как предлагалось ранее).

new List<string>(myDict.Values).ForEach(str =>
{
  //Use str in any other way you need here.
  Console.WriteLine(str);
});
Ник Л.
источник
0

Наряду с другими ответами, я подумал, что заметил бы, что если вы получите sortedDictionary.Keysили sortedDictionary.Valuesзатем переберите их foreach, вы также пройдете в отсортированном порядке. Это потому, что эти методы возвращают System.Collections.Generic.SortedDictionary<TKey,TValue>.KeyCollectionили SortedDictionary<TKey,TValue>.ValueCollectionобъекты, которые поддерживают вид исходного словаря.

СЭП
источник
0

Этот ответ предназначен для сравнения двух решений, а не предлагаемого решения.

Вместо того, чтобы создавать другой список, как предлагают другие ответы, вы можете использовать forцикл, используя словарь Countдля условия остановки цикла и Keys.ElementAt(i)для получения ключа.

for (int i = 0; i < dictionary.Count; i++)
{
    dictionary[dictionary.Keys.ElementAt(i)] = 0;
}

Во-первых, я думал, что это будет более эффективно, потому что нам не нужно создавать список ключей. После запуска теста я обнаружил, что forрешение по циклу намного менее эффективно. Причина в том, что ElementAtO (n) для dictionary.Keysсвойства, он ищет с начала коллекции, пока не дойдет до n-го элемента.

Тест:

int iterations = 10;
int dictionarySize = 10000;
Stopwatch sw = new Stopwatch();

Console.WriteLine("Creating dictionary...");
Dictionary<string, int> dictionary = new Dictionary<string, int>(dictionarySize);
for (int i = 0; i < dictionarySize; i++)
{
    dictionary.Add(i.ToString(), i);
}
Console.WriteLine("Done");

Console.WriteLine("Starting tests...");

// for loop test
sw.Restart();
for (int i = 0; i < iterations; i++)
{
    for (int j = 0; j < dictionary.Count; j++)
    {
        dictionary[dictionary.Keys.ElementAt(j)] = 3;
    }
}
sw.Stop();
Console.WriteLine($"for loop Test:     {sw.ElapsedMilliseconds} ms");

// foreach loop test
sw.Restart();
for (int i = 0; i < iterations; i++)
{
    foreach (string key in dictionary.Keys.ToList())
    {
        dictionary[key] = 3;
    }
}
sw.Stop();
Console.WriteLine($"foreach loop Test: {sw.ElapsedMilliseconds} ms");

Console.WriteLine("Done");

Полученные результаты:

Creating dictionary...
Done
Starting tests...
for loop Test:     2367 ms
foreach loop Test: 3 ms
Done
Элиаху Аарон
источник