Edición de valores de diccionario en un bucle foreach

192

Estoy tratando de construir un gráfico circular a partir de un diccionario. Antes de mostrar el gráfico circular, quiero ordenar los datos. Estoy eliminando cualquier rebanada de tarta que sería menos del 5% de la tarta y poniéndola en una rebanada de tarta "Otro". Sin embargo estoy obteniendo unCollection was modified; enumeration operation may not execute excepción en tiempo de ejecución.

Entiendo por qué no puede agregar o eliminar elementos de un diccionario mientras itera sobre ellos. Sin embargo, no entiendo por qué no puede simplemente cambiar un valor para una clave existente dentro del ciclo foreach.

Cualquier sugerencia con respecto a: arreglar mi código, sería 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
fuente

Respuestas:

262

Establecer un valor en un diccionario actualiza su "número de versión" interno, que invalida el iterador y cualquier iterador asociado con la colección de claves o valores.

Veo su punto, pero al mismo tiempo sería extraño si la colección de valores pudiera cambiar a mitad de la iteración, y para simplificar, solo hay un número de versión.

La forma normal de arreglar este tipo de cosas es copiar la colección de claves de antemano e iterar sobre la copia, o iterar sobre la colección original, pero mantener una colección de cambios que aplicará una vez que haya terminado de iterar.

Por ejemplo:

Copiar claves primero

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

O...

Crear una lista de modificaciones

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
fuente
24
Sé que esto es antiguo, pero si usa .NET 3.5 (¿o es 4.0?) Puede usar y abusar de LINQ de la siguiente manera: foreach (clave de cadena en colStates.Keys.ToList ()) {...}
Machtyn
66
@Machtyn: Claro, pero la pregunta era específicamente sobre .NET 2.0, de lo contrario, ciertamente habría usado LINQ.
Jon Skeet
¿Es el "número de versión" parte del estado visible del Diccionario o un detalle de implementación?
Cobarde anónimo
@SEinfringescopyright: no es visible directamente; Sin embargo, el hecho de que actualizar el diccionario invalida el iterador es visible.
Jon Skeet
Desde Microsoft Docs .NET framework 4.8 : La instrucción foreach es un contenedor alrededor del enumerador, que permite solo leer de la colección, no escribir en ella. Entonces diría que es un detalle de implementación que podría cambiar en una versión futura. Y lo que es visible es que el usuario del enumerador ha incumplido su contrato. Pero me equivocaría ... realmente es visible cuando se serializa un Diccionario.
Cobarde anónimo
81

Llame al ToList()en el foreachbucle. De esta manera no necesitamos una copia de la variable temporal. Depende de Linq, que está disponible 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;
    }
}
CAVAR
fuente
Muy buena mejora!
SpeziFish
1
Sería mejor usarlo foreach(var pair in colStates.ToList())para evitar tener acceso a la clave y al valor, lo que evita tener que recurrir a colStates[key]...
user2864740
21

Estás modificando la colección en esta línea:

colStates [clave] = 0;

Al hacerlo, esencialmente está eliminando y reinsertando algo en ese punto (en lo que respecta a IEnumerable de todos modos.

Si edita un miembro del valor que está almacenando, estaría bien, pero está editando el valor en sí mismo y a IEnumberable no le gusta eso.

La solución que he usado es eliminar el bucle foreach y simplemente usar un bucle for. Un bucle simple para no verificará los cambios que sabe que no afectarán la colección.

Así es como puedes hacerlo:

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
fuente
Me sale este problema usando for loop. dictionary [index] [key] = "abc", pero vuelve al valor inicial "xyz"
Nick Chan Abdullah
1
La solución en este código no es el ciclo for: está copiando la lista de claves. (Aún funcionaría si lo convirtieras en un bucle foreach). Resolver usando un bucle for significaría usar colStates.Keysen lugar de keys.
idbrii
6

No puede modificar las claves ni los valores directamente en ForEach, pero puede modificar sus miembros. Por ejemplo, esto debería 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
fuente
3

¿Qué tal simplemente hacer algunas consultas linq contra su diccionario y luego vincular su gráfico a los resultados de esos? ...

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
fuente
¿No está disponible Linq solo en 3.5? Estoy usando .net 2.0.
Aheho el
Puede usarlo desde 2.0 con una referencia a la versión 3.5 de System.Core.DLL; si eso no es algo que desea realizar, hágamelo saber y eliminaré esta respuesta.
Scott Ivey
1
Probablemente no iré por esta ruta, pero es una buena sugerencia, sin embargo. Le sugiero que deje la respuesta en su lugar en caso de que alguien con el mismo problema se encuentre con ella.
Aheho el
3

Si te sientes creativo, podrías hacer algo como esto. Recorra el diccionario hacia atrás para realizar sus cambios.

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

}

Ciertamente no es idéntico, pero de todos modos puede estar interesado ...

Hugoware
fuente
2

Debe crear un nuevo diccionario a partir del anterior en lugar de modificarlo en su lugar. Algo parecido (también iterar sobre KeyValuePair <,> en lugar de usar una búsqueda de clave:

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;
Ricardo
fuente
1

Comenzando con .NET 4.5 Puede hacer esto con 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);

Sin embargo, tenga en cuenta que su rendimiento es en realidad mucho peor que un simple 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
fuente
1

No puede modificar la colección, ni siquiera los valores. Puede guardar estos casos y eliminarlos más tarde. Terminaría así:

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
fuente
Usted puede modificar la colección. No puede seguir usando un iterador para una colección modificada.
user2864740
0

Descargo de responsabilidad: no hago mucho C #

Está intentando modificar el objeto DictionaryEntry que está almacenado en HashTable. Hashtable solo almacena un objeto: su instancia de DictionaryEntry. Cambiar la clave o el valor es suficiente para cambiar HashTable y hacer que el enumerador deje de ser válido.

Puedes hacerlo fuera del ciclo:

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

creando primero una lista de todas las claves de los valores que desea cambiar e iterar a través de esa lista.

Cambium
fuente
0

Puede hacer una copia de la lista de dict.Values, luego puede usar la List.ForEachfunción lambda para la iteración (o un foreachbucle, como se sugirió anteriormente).

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

Junto con las otras respuestas, pensé que notaría que si obtienes sortedDictionary.Keyso sortedDictionary.Valuesy luego las repites foreach, también lo harás en orden ordenado. Esto se debe a que esos métodos devuelven System.Collections.Generic.SortedDictionary<TKey,TValue>.KeyCollectionu SortedDictionary<TKey,TValue>.ValueCollectionobjetos, que mantienen el tipo del diccionario original.

jep
fuente
0

Esta respuesta es para comparar dos soluciones, no una solución sugerida.

En lugar de crear otra lista como sugieren otras respuestas, puede usar un forbucle usando el diccionario Countpara la condición de detención del bucle y Keys.ElementAt(i)para obtener la clave.

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

Al principio pensé que esto sería más eficiente porque no necesitamos crear una lista de claves. Después de ejecutar una prueba, descubrí que elfor solución de bucle es mucho menos eficiente. La razón es porque ElementAtes O (n) en eldictionary.Keys propiedad, busca desde el comienzo de la colección hasta llegar al enésimo elemento.

Prueba:

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
fuente