3

This seems like it should be answered but potential dupes I found were asking different things...

I noticed that this seems to work fine (sourceDirInclusion is a simple Dictionary<X,Y>)

    foreach (string dir in sourceDirInclusion.Keys)
    {
        if (sourceDirInclusion[dir] == null)
            sourceDirInclusion.Remove(dir);
    }

Does that mean removing items from a collection in foreach is safe, or that I got lucky?

What about if I was adding more elements to the dictionary rather than removing?

The problem I'm trying to solve is that sourceDirInclusion is initially populated, but then each value can contribute new items to the dictionary in a second pass. e.g what I want to do is like:

foreach (string dir in sourceDirInclusion.Keys)
{
  X x = sourceDirInclusion[dir];
  sourceDirInclusion.Add(X.dir,X.val);
}
Mr. Boy
  • 60,845
  • 93
  • 320
  • 589

4 Answers4

7

Short answer: This is not safe.

Long answer: From the IEnumerator<T> documentation:

An enumerator remains valid as long as the collection remains unchanged. If changes are made to the collection, such as adding, modifying, or deleting elements, the enumerator is irrecoverably invalidated and its behavior is undefined.

Note that the docs say the behavior is undefined, which means that it might work and it might not. One should never rely on undefined behavior.

In this case, it depends on the behavior of the Keys enumerable, regarding whether or not it creates a copy of the list of keys when you begin enumerating. In this specific case, we know from the docs that the return value from Dictionary<,>.Keys is a collection that refers back to the dictionary:

The returned Dictionary<TKey, TValue>.KeyCollection is not a static copy; instead, the Dictionary<TKey, TValue>.KeyCollection refers back to the keys in the original Dictionary<TKey, TValue>. Therefore, changes to the Dictionary<TKey, TValue> continue to be reflected in the Dictionary<TKey, TValue>.KeyCollection.

So it should be considered unsafe to modify the dictionary while enumerating the dictionary's keys.

You can correct this with one change. Alter this line:

foreach (string dir in sourceDirInclusion.Keys)

To this:

foreach (string dir in sourceDirInclusion.Keys.ToList())

The ToList() extension method will create an explicit copy of the list of keys, making it safe to modify the dictionary; the "underlying collection" will be the copy and not the original.

cdhowie
  • 158,093
  • 24
  • 286
  • 300
3

If will throw

InvalidOperationException: Message="Collection was modified; enumeration operation may not execute

To avoid that add candidates for removal to an external list. Then loop over it and remove from target container (dictionary).

List<string> list = new List<string>(sourceDirInclusion.Keys.Count);
foreach (string dir in sourceDirInclusion.Keys)
{
    if (sourceDirInclusion[dir] == null)
        list.Add(dir);
}
foreach (string dir in list)
{
    sourceDirInclusion.Remove(dir);
}
abatishchev
  • 98,240
  • 88
  • 296
  • 433
  • That's not actually my main question, but I guess the principle is exactly the same for adding as removing. – Mr. Boy Jan 24 '13 at 17:43
  • It will work the same, also your first example will throw an exception; if it's not it's because you are not getting any hits so you might want to rework that loop also. – Anthony Nichols Jan 24 '13 at 17:49
  • yeah, I suddenly realised my test cases probably never cover a `null` after writing this question saying it works. – Mr. Boy Jan 24 '13 at 17:51
  • Actually I could be wrong... As was pointed out by another member, you may actually be creating a new array of just the keys in your first `foreach` loop. – Anthony Nichols Jan 24 '13 at 17:59
0

check this out: What is the best way to modify a list in a 'foreach' loop?

In short:

The collection used in foreach is immutable. This is very much by design.

As it says on MSDN:

The foreach statement is used to iterate through the collection to get the information that you want, but can not be used to add or remove items from the source collection to avoid unpredictable side effects. If you need to add or remove items from the source collection, use a for loop.

UPDATE: You can use a for loop instead:

for (int index = 0; index < dictionary.Count; index++) {
  var item = dictionary.ElementAt(index);
  var itemKey = item.Key;
  var itemValue = item.Value;
}
Community
  • 1
  • 1
MUG4N
  • 19,377
  • 11
  • 56
  • 83
0

This works because you are traversing sourceDirInclusion.Keys.

However, just to be sure with future versions of the FrameWork I recommend that you use sourceDirInclusion.Keys.ToArray() in the foreach statement this way you will create a copy of the keys that you loop through.

This will however not work:

foreach(KeyValuePair<string, object> item in sourceDirInclusion)
{
    if (item.Value == null)
        sourceDirInclusion.Remove(item.Key);
}

As a rule, you cannot modify a collection while it is traversed, but often you can make a new collection by using .ToArray() or .ToList() and traverse that while modifying the original collection.

Good luck with your quest.

Casperah
  • 4,504
  • 1
  • 19
  • 13