Removing items from collection while iterating IEnumerable

Question

I have the following code:

foreach (var bar in dataFromDataFeed.Where(bar => bar.Key < fromTicks
                                                  || bar.Key > toTicks))
{
   dataFromDataFeed.Remove(bar.Key);
}

Is this safe or do I need to convert the IEnumerable within foreach to a Dictionary<T,U> first?

Thanks.

Answer 1

No, that will either bomb or get you a result that still has bad elements. Just invert the Where expression:

var filtered = dataFromDataFeed.Where(bar => bar.Key >= fromTicks && bar.Key <= toTicks);
dataFromFeed = filtered.ToList();   // optional

It isn't clear whether you actually need the list to be updated, often it is not necessary since you have a perfectly good enumerator, so the last statement is // optional.

Keep in mind that using Remove() like you did in your original code has O(n*m) complexity, very bad. Using ToList() is only O(m) but requires O(m) storage. Trading speed for memory is a common programmer's decision but this is a slamdunk unless m is huge (hundreds of millions and you're fighting OOM) or very small. Neither should apply, given the expression.

Answer 2

It is considered bad practice to modify a collection while it is being enumerated, and in many cases this will cause an InvalidOperationException to be thrown.

You should copy the values into another List or array (e.g. by calling ToList() after your Where() call), and then you won't be modifying the original data.

foreach (var bar in dataFromDataFeed.Where(bar => bar.Key < fromTicks || bar.Key > toTicks).ToList())
{
    dataFromDataFeed.Remove(bar.Key);
}

Answer 3

It will throw an error because you are iterating over the collection. Modification of collection is not supported while you are enumerating over it.

Create new List using ToList()

foreach (var bar in dataFromDataFeed.Where(bar => bar.Key < fromTicks
                                         || bar.Key > toTicks).ToList())
{
   dataFromDataFeed.Remove(bar.Key);
}

Answer 4

Although Dictionary could almost certainly have included a method which would all a predicate function on each item and remove all items where that predicate returned true, it doesn't. Consequently, if one has a Dictionary that includes some items which match a predicate and others which don't, the only ways to get a dictionary including only the items which don't satisfy the predicate are either to build a list of all the items satisfying the predicate and then remove from the dictionary all the items on the list, or else build a dictionary containing only items that don't satisfy the predicate and abandon the original in favor of the new one. Which approach is better will depend upon the relative numbers of items to be kept and discarded.

As an alternative, one could switch to using a ConcurrentDictionary. Unlike a Dictionary, a ConcurrentDictionary will allow items to be removed without invalidating any enumearation in progress. If one only removes items as they are enumerated, I would expect a ConcurrentDictionary to enumerate exactly as one would expects. If enumerating one item would sometimes cause code to delete a different item, then code must be prepared for the fact that removing an item which has not yet been enumerated might, but is not required to, cause that item to be omitted from the enumeration.

Although Dictionary is apt to generally be faster than ConcurrentDictionary, it may be worthwhile to use the latter if "delete items where..." operations are common and would have to either delete or copy a significant fraction of the items in the collection.