views:

252

answers:

1

I'm familiar with the problem of modifying a collection while looping over it with a foreach loop (i.e. "System.InvalidOperationException: Collection was modified"). However, it doesn't make sense to me that when I use Linq to create a List of keys to delete from a dictionary, then loop over my new List, I get the same exception.

Code before, that threw an exception:

IEnumerable<Guid> keysToDelete = _outConnections.Where(
    pair => pair.Value < timeoutPoint
).Select(pair => pair.Key);

foreach (Guid key in keysToDelete)
{
    ...some stuff not dealing with keysToDelete...
    _outConnections.Remove(key);
}

Code after, that worked:

List<Guid> keysToDelete = _outConnections.Where(
    pair => pair.Value < timeoutPoint
).Select(pair => pair.Key).ToList();

for (int i=keysToDelete.Count-1; i>=0; i--)
{
    Guid key = keysToDelete[i];
    ...some stuff not dealing with keysToDelete...
    _outConnections.Remove(key);
}

Why is this? I have the feeling that maybe my Linq queries aren't really returning a new collection, but rather some subset of the original collection, hence it accuses me of modifying the collection keysToDelete when I remove an element from _outConnections.

Update: the following fix also works, thanks to Adam Robinson:

List<Guid> keysToDelete = _outConnections.Where(
    pair => pair.Value < timeoutPoint
).Select(pair => pair.Key).ToList();

foreach (Guid key in keysToDelete)
{
    ...some stuff not dealing with keysToDelete...
    _outConnections.Remove(key);
}
+7  A: 

You're correct. LINQ uses what's called "deferred execution". Declaring your LINQ query doesn't actually do anything other than construct a query expression. It isn't until you actually enumerate over the list that the query is evaluated, and it uses the original list as the source.

However, calling ToList() should create a brand new list that has no relation to the original. Check the call stack of your exception to ensure that it is actually being thrown by keysToDelete.

Adam Robinson
Whoops, forgot that part of my fix involved adding the `ToList`. Originally, I just worked with the `IEnumerable`; updated question code to reflect this.
Sarah Vessels
Oho, based on your second paragraph, I could alter my fix to one of the following: 1) remove the `ToList` stuff and just use my `for` loop over the `IEnumerable` or 2) keep the `ToList` stuff and switch back to using a `foreach` loop since the new List is a separate collection. Thanks!
Sarah Vessels
You can't use the `for` option, since `IEnumerable` doesn't support index-based access. I would recommend switching to a `foreach` over the result of the `ToList()`.
Adam Robinson
Agreed. I don't like those simple `for` loops because they don't seem nearly as safe as a `foreach`. I added my new solution (`ToList` with `foreach`) to the question.
Sarah Vessels