Editing dictionary values in a foreach loop

Question

Setting a value in a dictionary updates its internal "version number" - which invalidates the iterator, and any iterator associated with the keys or values collection.

I do see your point, but at the same time it would be odd if the values collection could change mid-iteration - and for simplicity there's only one version number.

The normal way of fixing this sort of thing is to either copy the collection of keys beforehand and iterate over the copy, or iterate over the original collection but maintain a collection of changes which you'll apply after you've finished iterating.

For example:

Copying keys first

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

Or...

Creating a list of modifications

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

Call the ToList() in the foreach loop. This way we dont need a temp variable copy. It depends on Linq which is available since .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;
    }
}

You are modifying the collection in this line:

colStates[key] = 0;

By doing so, you are essentially deleting and reinserting something at that point (as far as IEnumerable is concerned anyways.

If you edit a member of the value you are storing, that would be OK, but you are editing the value itself and IEnumberable doesn't like that.

The solution I've used is to eliminate the foreach loop and just use a for loop. A simple for loop won't check for changes that you know won't effect the collection.

Here's how you could do it:

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

You can't modify the keys nor the values directly in a ForEach, but you can modify their members. E.g., this should work:

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

How about just doing some linq queries against your dictionary, and then binding your graph to the results of those?...

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

If you're feeling creative you could do something like this. Loop backwards through the dictionary to make your changes.

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

}

Certainly not identical, but you might be interested anyways...