Edycja wartości słownika w pętli foreach

192

Próbuję zbudować wykres kołowy ze słownika. Przed wyświetleniem wykresu kołowego chcę uporządkować dane. Usuwam wszystkie plasterki ciasta, które byłyby mniejsze niż 5% ciasta i umieszczam je w „innym” kawałku ciasta. Jednak dostaję Collection was modified; enumeration operation may not executewyjątek w czasie wykonywania.

Rozumiem, dlaczego nie można dodawać ani usuwać elementów ze słownika podczas iteracji nad nimi. Nie rozumiem jednak, dlaczego nie można po prostu zmienić wartości istniejącego klucza w pętli foreach.

Będziemy wdzięczni za wszelkie sugestie dotyczące: naprawy mojego kodu.

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
źródło

Odpowiedzi:

262

Ustawienie wartości w słowniku aktualizuje wewnętrzny „numer wersji” - co unieważnia iterator i każdy iterator powiązany z kolekcją kluczy lub wartości.

Rozumiem twój punkt widzenia, ale jednocześnie byłoby dziwne, gdyby zbiór wartości mógł zmienić środkową iterację - a dla uproszczenia jest tylko jeden numer wersji.

Normalnym sposobem naprawienia tego rodzaju rzeczy jest albo skopiowanie wcześniej zbioru kluczy i iteracja po kopii, albo iteracja po oryginalnej kolekcji, ale utrzymanie zbioru zmian, które zastosujesz po zakończeniu iteracji.

Na przykład:

Najpierw kopiowanie kluczy

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;
    }
}

Lub...

Tworzenie listy modyfikacji

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;
}
Jon Skeet
źródło
24
Wiem, że jest stary, ale jeśli używasz .NET 3.5 (czy jest to 4.0?), Możesz użyć LINQ w następujący sposób: foreach (klucz łańcucha w colStates.Keys.ToList ()) {...}
Machtyn
6
@Machtyn: Jasne - ale pytanie było konkretnie o .NET 2.0, w przeciwnym razie na pewno by wykorzystali LINQ.
Jon Skeet
Czy „numer wersji” jest częścią widocznego stanu Słownika lub szczegółem implementacji?
Anonimowy Tchórz
@SEinfringescopyright: Nie jest bezpośrednio widoczny; widoczny jest jednak fakt, że aktualizacja słownika unieważnia iterator .
Jon Skeet
Z Microsoft Docs .NET Framework 4.8 : Instrukcja foreach jest opakowaniem wokół modułu wyliczającego, który pozwala tylko czytać z kolekcji, a nie pisać do niej. Powiedziałbym więc, że jest to szczegół implementacji, który może ulec zmianie w przyszłej wersji. Widoczne jest, że użytkownik modułu wyliczającego naruszył umowę. Ale myliłbym się ... to naprawdę widać, gdy słownik jest serializowany.
Anonimowy Tchórz
81

Zadzwoń ToList()w foreachpętli. W ten sposób nie potrzebujemy kopii zmiennej temp. To zależy od Linq, który jest dostępny od wersji .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;
    }
}
KOPAĆ
źródło
Bardzo fajna poprawa!
SpeziFish,
1
Lepiej byłoby użyć, foreach(var pair in colStates.ToList())aby uniknąć dostępu do Klucza i Wartości, co pozwala uniknąć konieczności wywoływania colStates[key]..
user2864740
21

Modyfikujesz kolekcję w tym wierszu:

colStates [klucz] = 0;

Robiąc to, zasadniczo usuwasz i ponownie wstawiasz coś w tym momencie (jeśli chodzi o IEnumerable i tak.

Jeśli edytujesz członka przechowywanej wartości, byłoby to w porządku, ale edytujesz samą wartość, a IEnumberable tego nie lubi.

Rozwiązaniem, które zastosowałem, jest wyeliminowanie pętli foreach i użycie pętli for. Prosta pętla for nie sprawdza zmian, o których wiesz, że nie wpłyną na kolekcję.

Oto jak możesz to zrobić:

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
źródło
Ten problem pojawia się przy użyciu pętli for. słownik [indeks] [klucz] = "abc", ale wraca do wartości początkowej "xyz"
Nick Chan Abdullah
1
Poprawka w tym kodzie nie dotyczy pętli for: kopiuje listę kluczy. (Wciąż działałoby, jeśli przekształciłeś go w pętlę foreach.) Rozwiązanie za pomocą pętli for oznaczałoby użycie colStates.Keyszamiast keys.
idbrii
6

Nie możesz modyfikować kluczy ani wartości bezpośrednio w ForEach, ale możesz modyfikować ich członków. Np. Powinno to działać:

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 } );
Jeremy Frey
źródło
3

Co powiesz na wykonanie kilku zapytań linq na słownik, a następnie powiązanie wykresu z ich wynikami? ...

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);
}
Scott Ivey
źródło
Czy Linq jest dostępny tylko w wersji 3.5? Używam .net 2.0.
Aheho,
Możesz użyć go w wersji 2.0 w odniesieniu do wersji 3.5 System.Core.DLL - jeśli nie jest to coś, co chciałbyś podjąć, daj mi znać, a ja usunę tę odpowiedź.
Scott Ivey
1
Prawdopodobnie nie wybiorę tej trasy, ale mimo to jest to dobra sugestia. Sugeruję pozostawienie odpowiedzi na miejscu, na wypadek, gdyby ktoś inny z tym samym problemem natknął się na nią.
Aheho,
3

Jeśli czujesz się kreatywny, możesz zrobić coś takiego. Pętlę do tyłu w słowniku, aby wprowadzić zmiany.

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)); ;
    }

}

Na pewno nie identyczne, ale i tak możesz być zainteresowany ...

Hugoware
źródło
2

Musisz utworzyć nowy Słownik ze starego, a nie modyfikować w miejscu. Coś w rodzaju (iteruj również nad KeyValuePair <,> zamiast wyszukiwania klucza:

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;
Richard
źródło
1

Począwszy od .NET 4.5 Możesz to zrobić za pomocą 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);

Zauważ jednak, że jego wydajność jest znacznie gorsza niż prosta 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>();
    }
}

Wynik:

              Method |      Mean |     Error |    StdDev |
--------------------- |----------:|----------:|----------:|
 ConcurrentDictionary | 182.24 us | 3.1507 us | 2.7930 us |
           Dictionary |  47.01 us | 0.4824 us | 0.4512 us |
Ohad Schneider
źródło
1

Nie możesz modyfikować kolekcji, nawet wartości. Możesz zapisać te skrzynki i usunąć je później. Skończy się tak:

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);
Samuel Carrijo
źródło
Państwo może modyfikować kolekcji. Nie można używać iteratora do zmodyfikowanej kolekcji.
user2864740,
0

Oświadczenie: nie robię dużo C #

Próbujesz zmodyfikować obiekt DictionaryEntry, który jest przechowywany w HashTable. Hashtable przechowuje tylko jeden obiekt - twoją instancję DictionaryEntry. Zmiana klucza lub wartości wystarczy, aby zmienić tabelę mieszania i spowodować, że moduł wyliczający stanie się nieprawidłowy.

Możesz to zrobić poza pętlą:

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

najpierw tworząc listę wszystkich kluczy wartości, które chcesz zmienić, i zamiast tego powtarzaj tę listę.

Kambium
źródło
0

Możesz wykonać kopię listy dict.Values, a następnie możesz użyć List.ForEachfunkcji lambda do iteracji (lub foreachpętli, jak sugerowano wcześniej).

new List<string>(myDict.Values).ForEach(str =>
{
  //Use str in any other way you need here.
  Console.WriteLine(str);
});
Nick L.
źródło
0

Wraz z innymi odpowiedziami, pomyślałem, że pamiętać, że jeśli masz sortedDictionary.Keyslub sortedDictionary.Valuesa następnie pętli nad nimi foreach, można również przejść posortowanych. Jest tak, ponieważ metody te zwracają System.Collections.Generic.SortedDictionary<TKey,TValue>.KeyCollectionlub SortedDictionary<TKey,TValue>.ValueCollectionobiekty, które zachowują rodzaj oryginalnego słownika.

jep
źródło
0

Ta odpowiedź służy do porównania dwóch rozwiązań, a nie sugerowanego rozwiązania.

Zamiast tworzyć kolejną listę, jak sugerują inne odpowiedzi, można użyć forpętli za pomocą słownika Countdla warunku zatrzymania pętli i Keys.ElementAt(i)uzyskania klucza.

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

W firs myślałem, że będzie to bardziej wydajne, ponieważ nie musimy tworzyć listy kluczy. Po uruchomieniu testu stwierdziłem, że forrozwiązanie pętli jest znacznie mniej wydajne. Powodem jest to, że ElementAtjest O (n) nadictionary.Keys właściwości , szuka ona od początku kolekcji, aż dojdzie do n-tego elementu.

Test:

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");

Wyniki:

Creating dictionary...
Done
Starting tests...
for loop Test:     2367 ms
foreach loop Test: 3 ms
Done
Eliahu Aaron
źródło