23

I have two collections of strings: CollectionA is a StringCollection property of an object stored in the system, while CollectionB is a List generated at runtime. CollectionA needs to be updated to match CollectionB if there are any differences. So I devised what I expected to be a simple LINQ method to perform the removal.

var strDifferences = CollectionA.Where(foo => !CollectionB.Contains(foo));
foreach (var strVar in strDifferences) { CollectionA.Remove(strVar); }

But I am getting a "Collection was modified; enumeration operation may not execute" error on strDifferences... even though it is a separate enumerable from the collection being modified! I originally devised this explicitly to evade this error, as my first implementation would produce it (as I was enumerating across CollectionA and just removing when !CollectionB.Contains(str)). Can anyone shed some insight into why this enumeration is failing?

Grace Note
  • 3,205
  • 4
  • 35
  • 55

3 Answers3

33

The two are not completely separate. Where does not make a separate copy of the collection. It internally keeps a reference to the original collection and fetches elements from it as you request them.

You can solve your problem by adding ToList() to force Where to iterate through the collection immediately.

var strDifferences = CollectionA
    .Where(foo => !CollectionB.Contains(foo))
    .ToList();
Mark Byers
  • 811,555
  • 193
  • 1,581
  • 1,452
  • Wouldn't `ToArray()` be better here? There is no need to use `List`. – svick May 07 '10 at 21:22
  • @svick That's something I've wondered about for quite some time, but that'd be a question I'd ask on another day. Provided it hasn't already been asked, of course -chuckle- – Grace Note May 07 '10 at 21:24
  • 2
    @GraceNote catch that here: http://stackoverflow.com/questions/1105990/is-it-better-to-call-tolist-or-toarray-in-linq-queries – nawfal Jun 05 '13 at 14:27
4

Where passes back IEnumerable<T> and then you are using that in your foreach (var strVar in strDifferences)

You are then trying to remove it from the collection that created the IEnumerable<T>. You haven't created a new list, it's referencing CollectionA to pull the next item from, so you can't edit CollectionA.

You can do this also:

var strDifferences = CollectionA.Where
  (foo => CollectionB.Contains(foo)).ToList();

CollectionA = strDifferences;
//or instead of reassigning CollectionA
CollectionA.Clear();
CollectionA.AddRange(strDifferences);

Since you are removing the ones that aren't in CollectionB. Just look for the ones that are, create a list and assign that list to the CollectionA variable.

kemiller2002
  • 113,795
  • 27
  • 197
  • 251
  • Ah... I'd love to use your alternative (however identical in effect it is), but unfortunately CollectionA, it turns out, is a read-only property, so I can't set it. You do give a much more in-depth explanation, though, so +1 for that. – Grace Note May 07 '10 at 20:50
  • @ccornet if can't set it you could clear it and add the items back in. – kemiller2002 May 07 '10 at 21:00
  • I thought about it long and hard. I decided to still go with the removal approach. I don't wish to modify CollectionA if no change is necessary, building a list of what needs to be removed makes this very quick and simple to know (just see if strDifferences.Count > 0). If I build an array of the goal, I'd still have to check if CollectionA has something that needs to be removed. – Grace Note May 10 '10 at 12:53
3

Try modifying the first line a bit:

var strDifferences =
    CollectionA.Where(foo => !CollectionB.Contains(foo)).ToList();

I think LINQ is using lazy execution of your query. The call to ToList will force execution of the query prior to enumerating.

Justin Niessner
  • 242,243
  • 40
  • 408
  • 536
  • Yes, and make sure not to put it over two lines i..e. var strDifferencesTemp = CollectionA.Where(foo => !CollectionB.Contains(foo)); var strDifferences = strDifferencesTemp.ToList(); if you do so, the collection can be modified by some other thread between these two lines. – Markus Jun 02 '17 at 16:48
  • @Markus if multi-threading, it doesn't make any difference even you put it in one line. You'd better consider immutablelist in that case. – joe Nov 04 '19 at 08:16