views:

11910

answers:

10

I'm trying to update a hashtable in a loop but getting an error: System.InvalidOperationException: Collection was modified; enumeration operation may not execute.

private Hashtable htSettings_m = new Hashtable();
htSettings_m.Add("SizeWidth", "728");
htSettings_m.Add("SizeHeight", "450");
string sKey = "";
string sValue = "";
foreach (DictionaryEntry deEntry in htSettings_m)
{
    // Get value from Registry and assign to sValue.
    // ...
    // Change value in hashtable.
    sKey = deEntry.Key.ToString();
    htSettings_m[sKey] = sValue;
}

Is there way around it or maybe there is a better data structure for such purpose?

+2  A: 

You cannot change the set of items stored in a collection while you are enumerating over it, since that makes life very difficult for the iterator in most cases. Consider the case where the collection represents a balanced tree, and may well undergo rotations after an insert. The enumerate would have no plausible way of keeping track of what it has seen.

However, if you are just trying to update the value then you can write:

deEntry.Value = sValue

Updating the value here has no impact on the enumerator.

Rob Walker
That doesn't compile: Cannot modify members of 'deEntry' because it is a 'foreach iteration variable'
z-boss
+2  A: 

In concept I would do:

Hashtable table = new Hashtable(); // ps, I would prefer the generic dictionary..
Hashtable updates = new Hashtable();

foreach (DictionaryEntry entry in table)
{
   // logic if something needs to change or nog
   if (needsUpdate)
   {
      updates.Add(key, newValue);
   }
}

// now do the actual update
foreach (DictionaryEntry upd in updates)
{
   table[upd.Key] = upd.Value;
}
Davy Landman
Good solution as well. Thanks.
z-boss
A: 

Maybe you can use Hashtable.Keys collection? Enumerating through that might be possible while changing the Hashtable. But it's only a guess...

Vilx-
No, that doesn't work.
z-boss
+7  A: 

you could read the collection of keys into another IEnumerable instance first, then foreach over that list

        System.Collections.Hashtable ht = new System.Collections.Hashtable();

        ht.Add("test1", "test2");
        ht.Add("test3", "test4");

        List<string> keys = new List<string>();
        foreach (System.Collections.DictionaryEntry de in ht)
            keys.Add(de.Key.ToString());

        foreach(string key in keys)
        {
            ht[key] = DateTime.Now;
            Console.WriteLine(ht[key]);
        }
keithwarren7
This one will do. Thanks!
z-boss
A: 
private Hashtable htSettings_m = new Hashtable();

htSettings_m.Add("SizeWidth", "728");    
htSettings_m.Add("SizeHeight", "450");    
string sValue = "";    
foreach (string sKey in htSettings_m.Keys)    
{    
    // Get value from Registry and assign to sValue    
    // ...    
    // Change value in hashtable.    
    htSettings_m[sKey] = sValue;    
}
Stephen Martin
Produces the same error.
z-boss
That's the problem with responding from faulty memory without testing first. I thought about this again and I remembered that Hashtable uses the same enumerator type for a Hashtable and for it's Keys. A seriously flawed implementation in my opinion.
Stephen Martin
No, the problem isn't the enumerator type - it's that the Keys property doesn't take a *copy* of all the keys, it just iterates over the underlying collection. When you *need* to take a copy, do so explicitly. It's the behaviour I want and expect, personally.
Jon Skeet
I have to disagree with you here: the Keys collection is the Keys collection not the Hashtable, the Keys enumerator should be sensitive to changes in the Keys collection only, not in the Hashtable as a whole.
Stephen Martin
I think this is a design defect in the Hashtable class. See more details in my answer.
Robert Rossney
A: 

It depends on why you are looping through the items in the hashtable. But you would probably be able to iterate throught the keys instead. So

    foreach (String sKey in htSettings_m.Keys)
{    // Get value from Registry and assign to sValue.
    // ...    // Change value in hashtable.
htSettings_m[sKey] = sValue;

}

The other option is to create a new HashTable. Iterate through the first while adding items to the second then replace the original with the new one.
Looping through the keys requires less object allocations though.

pipTheGeek
+2  A: 

The simplest way is to copy the keys into a separate collection, then iterate through that instead.

Are you using .NET 3.5? If so, LINQ makes things a little bit easier.

Jon Skeet
+1  A: 

If you're using a Dictionary instead of a Hashtable, so that the type of the keys is known, the easiest way to make a copy of the Keys collection to avoid this exception is:

foreach (string key in new List<string>(dictionary.Keys))

Why are you getting an exception telling you that you've modified the collection you're iterating over, when in fact you haven't?

Internally, the Hashtable class has a version field. The Add, Insert, and Remove methods increment this version. When you create an enumerator on any of the collections that the Hashtable exposes, the enumerator object includes the current version of the Hashtable. The enumerator's MoveNext method checks the enumerator's version against the Hashtable's, and if they're not equal, it throws the InvalidOperationException you're seeing.

This is a very simple mechanism for determining whether or not the Hashtable has been modified. In fact it's a little too simple. The Keys collection really ought to maintain its own version, and its GetEnumerator method ought to save the collection's version in the enumerator, not the Hashtable's version.

There's another, subtler design defect in this approach. The version is an Int32. The UpdateVersion method does no bounds checking. It's therefore possible, if you make exactly the right number of modifications to the Hashtable (2 times Int32.MaxValue, give or take), for the version on the Hashtable and the enumerator to be the same even though you've radically changed the Hashtable since creating the enumerator. So the MoveNext method won't throw the exception even though it should, and you'll get unexpected results.

Robert Rossney
+1  A: 

The key part is the ToArray() method

var dictionary = new Dictionary<string, string>();
foreach(var key in dictionary.Keys.ToArray())
{
 dictionary[key] = "new value";
}
Gian Marco Gherardi
A: 

this is how I did it within a dictionary; resets every value in dict to false:

Dictionary dict = new Dictionary();

for (int i = 0; i < dict.Count; i++) { string key = dict.ElementAt(i).Key; dict[key] = false; }

Okan