Editando valores de dicionário em um loop foreach

191

Estou tentando criar um gráfico de pizza a partir de um dicionário. Antes de exibir o gráfico de pizza, quero arrumar os dados. Estou removendo as fatias de torta que seriam inferiores a 5% da torta e colocando-as em uma fatia de "Outros". No entanto, estou recebendo uma Collection was modified; enumeration operation may not executeexceção em tempo de execução.

Entendo por que você não pode adicionar ou remover itens de um dicionário enquanto itera sobre eles. No entanto, não entendo por que você não pode simplesmente alterar um valor para uma chave existente no loop foreach.

Qualquer sugestão re: corrigir o meu código, seria apreciada.

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
fonte

Respostas:

259

Definir um valor em um dicionário atualiza seu "número da versão" interno - o que invalida o iterador e qualquer iterador associado à coleção de chaves ou valores.

Entendo seu ponto de vista, mas ao mesmo tempo seria estranho se a coleção de valores pudesse mudar no meio da iteração - e por simplicidade, existe apenas um número de versão.

A maneira normal de corrigir esse tipo de coisa é copiar a coleção de chaves antecipadamente e iterar sobre a cópia ou iterar sobre a coleção original, mas manter uma coleção de alterações que você aplicará após concluir a iteração.

Por exemplo:

Copiando chaves primeiro

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

Ou...

Criando uma lista de modificações

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
fonte
24
Eu sei que isso é antigo, mas se você estiver usando o .NET 3.5 (ou é 4.0?), Poderá usar e abusar do LINQ da seguinte maneira: foreach (chave de seqüência de caracteres em colStates.Keys.ToList ()) {...}
Machtyn
6
@ Machtyn: Claro - mas a pergunta era especificamente sobre o .NET 2.0, caso contrário eu certamente teria usado o LINQ.
Jon Skeet
O "número da versão" faz parte do estado visível do dicionário ou é um detalhe de implementação?
Covarde Anônimo
@SEinfringescopyright: não é visível diretamente; o fato de que a atualização do dicionário invalide o iterador é visível.
Jon Skeet
No Microsoft Docs .NET framework 4.8 : A instrução foreach é um invólucro em torno do enumerador, que permite apenas a leitura da coleção e não a gravação. Então, eu diria que é um detalhe de implementação que pode mudar em uma versão futura. E o que é visível é que o usuário do Enumerador violou seu contrato. Mas eu estaria errado ... é realmente visível quando um Dicionário é serializado.
Covarde Anônimo
81

Ligue ToList()para o foreachloop. Dessa forma, não precisamos de uma cópia de variável temporária. Depende do Linq, que está disponível desde .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;
    }
}
ESCAVAÇÃO
fonte
Melhoria muito boa!
SpeziFish
1
Seria melhor usar foreach(var pair in colStates.ToList())para evitar ter acesso à chave e o valor que evita a necessidade de pôr em colStates[key]..
user2864740
21

Você está modificando a coleção nesta linha:

colStates [chave] = 0;

Ao fazer isso, você está essencialmente excluindo e reinserindo algo nesse ponto (no que diz respeito ao IEnumerable de qualquer maneira.

Se você editar um membro do valor que está armazenando, tudo bem, mas você está editando o próprio valor e IEnumberable não gosta disso.

A solução que usei é eliminar o loop foreach e usar um loop for. Um loop for simples não verificará as alterações que você sabe que não afetarão a coleção.

Veja como você pode fazer isso:

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
fonte
Eu recebo esse problema usando o loop for. dictionary [index] [key] = "abc", mas volta ao valor inicial "xyz"
Nick Chan Abdullah
1
A correção neste código não é o loop for: está copiando a lista de chaves. (Ainda funcionaria se você o convertesse em um loop foreach.) Resolver usando um loop for significaria usar colStates.Keysno lugar de keys.
Idbrii 27/03/19
6

Você não pode modificar as chaves nem os valores diretamente em um ForEach, mas pode modificar seus membros. Por exemplo, isso deve funcionar:

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
fonte
3

Que tal fazer algumas consultas linq no seu dicionário e vincular seu gráfico aos resultados deles? ...

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
fonte
O Linq não está disponível apenas no 3.5? Estou usando .net 2.0.
Aheho 01/07/2009
Você pode usá-lo a partir do 2.0 com uma referência à versão 3.5 do System.Core.DLL - se isso não é algo que você gostaria de empreender, avise-me e eu excluirei esta resposta.
911 Scott Scott Ivey
1
Provavelmente não vou seguir esse caminho, mas é uma boa sugestão. Sugiro que você deixe a resposta no lugar, caso outra pessoa com o mesmo problema tropeça nela.
Aheho 03/07/2009
3

Se você estiver se sentindo criativo, poderá fazer algo assim. Faça um retrocesso no dicionário para fazer suas alterações.

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

}

Certamente não é idêntico, mas você pode estar interessado de qualquer maneira ...

Hugoware
fonte
2

Você precisa criar um novo dicionário a partir do antigo, em vez de modificar no local. Algo parecido (também itere sobre KeyValuePair <,> em vez de usar uma pesquisa de chave:

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
fonte
1

Iniciando com o .NET 4.5 Você pode fazer isso com o 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);

Observe, no entanto, que seu desempenho é realmente muito pior que um simples 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>();
    }
}

Resultado:

              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
fonte
1

Você não pode modificar a coleção, nem mesmo os valores. Você pode salvar esses casos e removê-los mais tarde. Terminaria assim:

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
fonte
Você pode modificar a coleção. Você não pode continuar usando um iterador para uma coleção modificada.
user2864740
0

Isenção de responsabilidade: eu não faço muito c #

Você está tentando modificar o objeto DictionaryEntry que é armazenado no HashTable. O Hashtable armazena apenas um objeto - sua instância do DictionaryEntry. Alterar a chave ou o valor é suficiente para alterar a HashTable e fazer com que o enumerador se torne inválido.

Você pode fazer isso fora do loop:

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

criando primeiro uma lista de todas as chaves dos valores que você deseja alterar e iterar por essa lista.

Câmbio
fonte
0

Você pode fazer uma cópia da lista e dict.Values, em seguida, usar a List.ForEachfunção lambda para iteração (ou um foreachloop, conforme sugerido anteriormente).

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

Juntamente com as outras respostas, pensei em observar que, se você obtiver sortedDictionary.Keysou, em sortedDictionary.Valuesseguida foreach, passar por cima delas , você também será ordenado. Isso ocorre porque esses métodos retornam System.Collections.Generic.SortedDictionary<TKey,TValue>.KeyCollectionou SortedDictionary<TKey,TValue>.ValueCollectionobjetos, que mantêm o tipo do dicionário original.

jep
fonte
0

Esta resposta é para comparar duas soluções, não uma solução sugerida.

Em vez de criar outra lista conforme as outras respostas sugeridas, você pode usar um forloop usando o dicionário Countpara a condição de parada de loop e Keys.ElementAt(i)obter a chave.

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

No começo, pensei que isso seria mais eficiente, porque não precisamos criar uma lista de chaves. Depois de executar um teste, descobri que a forsolução de loop é muito menos eficiente. O motivo é porque ElementAtO (n) está nodictionary.Keys propriedade, ele pesquisa desde o início da coleção até chegar ao enésimo item.

Teste:

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

Resultados:

Creating dictionary...
Done
Starting tests...
for loop Test:     2367 ms
foreach loop Test: 3 ms
Done
Eliahu Aaron
fonte