See the following code:
List<int> list = new List<int>();
for (int i = 1; i < 10; i++)
{
list.Add(i);
}
foreach (int i in list)
{
list.Remove(i);
}
Do you see anything wrong in this piece of code? No? Okay.
Execute this code and you will get a System.InvalidOperationException with the message “Collection was modified; enumeration operation may not execute.”
The first time foreach loop executes, it deletes the first item and in the second iteration, this nasty exception is thrown. Reason? You deleted the first item and the list has changed. If not for the InvalidOperationException , you could have got a IndexOutOfRange exception while traversing the list.
So, how does one delete some items in the list while iterating through the list? I faced this problem in my project today and I couldn’t come up with an elegant solution. I did come up with a hack. I identify all items that need to be deleted and mark them with a flag (I need one iteration here). In another iteration, I create a new list and copy all those items which are not marked.
Sorry, I couldn’t come up with something better. If you have a solution or a better fix, please do let me know. Please note that if I want to remove all items, I can use list.Clear(). I am talking about a scenario where I want to delete only a select few items.
Update:
I read somewhere that if you use for loop instead of foreach, this problem does not arise. True, you will not get an exception, but you might not delete the items that you actually want to delete. Let’s see what happens in our case.
List<int> list = new List<int>();
for (int i = 1; i < 10; i++)
{
list.Add(i);
}
for (int i = 0; i < list.Count; i++)
{
int remove = list[i];
list.Remove(remove);
}
You will delete items 1, 3, 5, 7 and 9. After the for loop exits, your list will still have 2, 4, 6 and 8. Surprised?
Let’s see why this is so.
First iteration: i = 0, remove = 1. Deleted item = 1 Once you delete this item, the list has changed. So, the first element in the list is now 2.
Second iteration: i = 1, remove = 3. Deleted item = 3
Third iteration: i = 2, remove = 5. Deleted item = 5
and so on….
So, replacing foreach with a for is not a solution.
October 26, 2007 at 9:42 am
Can you not use the ‘for’ method but loop backwards over the list removing them one at a time?
This should overcome the elements shifting into the position of those deleted.
October 26, 2007 at 11:04 am
Thanks for the solution, Tony.
See another solution here: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=2314310&SiteID=1
October 31, 2007 at 8:53 pm
That’s what I do.
for(int i=10 ; i>0 ; i–)
Remove(i);
November 2, 2007 at 5:33 pm
For most cases like this, you can use delegates:
//removes all even numbers from list:
NumberList.RemoveAll(delegate(int x) { return x%2 == 0; });
November 20, 2007 at 6:03 am
Reverse-iteration is, traditionally, the “proper” way to do a For-loop that removes elements from the list.
Personally, though, I tend to just use delegates, if you’re lucky enough to be using the List class.
March 24, 2008 at 8:15 am
What I end up doing is making a copy of the list, removing the needed items from the copy, and then replacing the original with the copy to avoid this problem.
April 3, 2008 at 10:27 pm
You can also just delete entry 0 (zero) in the for loop instead of deleting the i entry. Each time you look and remove index=0, you will create a “new” zero entry. You could of course set the count to zero…
April 4, 2008 at 9:44 am
gjunky, this works if I want to delete all the items in the list. What if I want to delete some items depending on some condition?
April 7, 2008 at 12:34 am
I use the more elegant “black list”:
List remList = new List();
foreach (TestClass tc in list)
if (hasToBeDeleted) remList.Add(tc);
list.RemoveAll(new Predicate(delegate(TestClass x) { return remList.Contains(x);}));
April 18, 2008 at 11:20 pm
I use a different approach, create a second list that “filtter” the items that must be removed and then replace the original list with the new one.
Basically is like “filtering” instead of “removing”, the cons is that you must recreate the list but I think the Add operation for List must not be so heavy.
Here is an example (observe the ! preceding the condition that indicates which items must be removed, it is negative logic!):
//Orignal List
List list = new List();
for (int i = 1; i < 10; i++)
{
list.Add(i);
}
//Filtered List
List newList = new List();
foreach (int i in list)
{
if (! MustRemove(i))
newList.Add(i);
}
//Replacing the original list
list = newList;
Regards.
May 5, 2008 at 7:52 pm
I like this gentleman’s approach; wrap it up ina isloator class that lets you modify the items of the list while iterating it:
http://www.koders.com/csharp/fidF2C8E1786AE76101911914CCA9286E25B398AED6.aspx
May 21, 2008 at 5:33 am
What I do is, during the first iteration, I get the lndexes of elements that i want to remove and then call the following function :
private void removeElementsFromList(List list, List indexOfElementsToBeRemoved)
{
try
{
Collections.sort(indexOfElementsToBeRemoved);
Iterator indexes = indexOfElementsToBeRemoved.iterator();
int counter = 0;
while(indexes.hasNext())
{
int updatedIndexOfItemToBeRemoved = indexes.next() – counter;
list.remove(updatedIndexOfItemToBeRemoved);
counter++;
}
}
catch (Exception e)
{
}
}
June 19, 2008 at 7:47 am
I have a recommendation. Borrowing from the other site in question, I recommend this approach:
int t = 0;
object o;
while ( t < list.count )
{
o = list[t];
if ( o.shouldDelete() )
list.remove(o);
else
t++;
}
You’ll either run out of elements or get to the end of the list.
July 12, 2008 at 9:24 pm
maybe list.Clear();
?
July 25, 2008 at 1:38 pm
Using the for solution, only increment if you didn’t remove.
August 1, 2008 at 7:45 pm
I think Josh’s solution using a delegate is the most elegant. However, if delegates are not an option, why not iterate over a copy of the original list? That way you’d come up with something looking pretty close to what you originally wanted to do …
foreach (int i in new List(list))
{
list.Remove(i);
}
Far more readable than the “black list” approaches above IMHO and you can avoid having to manually maintain a list index until you really need to for performance reasons.
August 1, 2008 at 7:51 pm
Oops, the generic list declaration was stripped by WordPress … I’ll try again
foreach (int i in new List<int>(list))
{
list.Remove(i);
}
October 27, 2008 at 5:48 pm
It’s easier than that. This works for both foreach and for loops.
// Create 2 lists.
new List
new ListDeletions
// Iterate through the loop
For each item in list
if condition is met add this to listDeletions
// When the loop is finished update List
List.Except(ListDeletions)
It’s called Linq and it works for removing any specific entries in any order
What you guys are talking about is a FIFO list aka. Queue. You could do like a previous comment said. Create a for loop and loop through the entries in reverse order removing the items after they have been processed. The downside of this is is creates a LIFO list aka. stack. So, to reverse it, you’d have to duplicate the original list in the reverse order to actually create a queue.
I can vouch that both these methods work because I have used both. If you use the for Stack it is more performant but less flexible.
There are also specific classes that are tied into iEnumerable. Called Stack and Queue (go figure).
You can find specifics on how to use queue here:
http://www.blackwasp.co.uk/Queue.aspx
And how to use the stack here:
http://www.blackwasp.co.uk/Stack.aspx
There’s also a Freemind MindMap for C# Here:
freemind.sourceforge.net/wiki/extensions/freemind/flashwindow.php?startCollapsedToLevel=3&initLoadFile=/wiki/images/e/ee/CSharp_2_WebLinks2.mm&mm_title=C%23%202.0%20Computer%20Language%20-%20Contents
You can find it in the MindMap under C# 2.0 Collections
There’s no need to re-invent the wheel, unless you need a custom implementation that performs better.
If you work with iEnumerables a lot I suggest you learn Linq-To-Objects, it will save you an untold amount of dev time. Check out [2008 – Apress – LINQ For Visual C Sharp 2008. It’s the only decent book resource I’ve found on the subject so far.
November 28, 2008 at 8:35 pm
delegates are definately the best approach and now you have the benefit of lamda expressions which are even more concise and elegant.
February 14, 2009 at 9:56 pm
what about for dictionary?
you can’t normal for() loop over it, apparently
February 21, 2009 at 9:57 am
thanks for the delegate code here… ive never used it befre and i just did! I am writing a game in C# and XNA and keeping track of bullets fired and managed by my ObjectManager class. This allows me to delete ‘dead’ projectiles instead of reusing them- thanks for the idea!
May 15, 2009 at 2:39 am
The issue I had was editing an object in the list, and possibly removing it on a condition. Not sure if a delegate would work. I did as follows:
List MyList;
ObjClass [] temp = new ObjClass[MyList.Count];
ObjClass.CopyTo(temp);
foreach (ObjClass obj in temp)
{
//if condition met, modifiy obj properties
obj.x = … obj.y = …
//if other condition met, remove obj
MyList.Remove(obj);
}
June 1, 2009 at 9:23 pm
It is as easy as unbinding the list from foreach. Just add ToArray() to the list
List list = new List();
for (int i = 1; i < 10; i++)
{
list.Add(i);
}
foreach (int i in list.ToArray())
{
list.Remove(i);
}
June 25, 2009 at 6:52 pm
Yup this works… It’s like Splice implementation, which perhaps resets the Enumerators….
June 19, 2009 at 11:06 pm
The reverse for loop is the fastest method:
for (int idx = list.Count – 1; idx >= 0; idx–)
{
if (list[idx].Remove()) list.RemoveAt(idx);
}
1) The Count property is called only once. In a foward loop it would be called after each iteration, or you would have to assign it to a variable before hand.
2) RemoveAt() has better performance than Remove().
3) Though I wouldn’t know for sure, my instinct is that ForEach is slower that For.
4) Obviously solutions that copy lists to other lists, will be adding all sorts of weight to the procedure. And though it doesn’t appear to be the case, methods like list.ToArray() are doing just that.
It’s not that ugly in my opinion.
June 30, 2009 at 7:49 pm
To remove selected item in multiselect list box:
for (int i=lbx_ProdMaster.Items.Count ; i>0 ; i–)
{
if(lbx_ProdMaster.Items[i-1].Selected == true)
{
lbx_ProdMaster.Items.RemoveAt(i-1);
}
}
July 14, 2009 at 4:59 pm
You can use lambda expressions as shown here
http://my6solutions.com/post/2009/07/13/Collection-was-modified3b-enumeration-operation-may-not-execute.aspx
August 15, 2009 at 10:02 am
This annoys me to no end.
The proper (imho) way to do this is to use a LinkedList (either use the default one or roll your own). This kind of dynamic modification is what linked structures were designed for.
However, there really SHOULD be a higher level abstraction for doing this kind of enumeration.
The “for each” syntax is very elegant to use, but its associated semantics are too restrictive. I think it should work the same way a linked list works, which is that you can make any modifications you want to the list while enumerating, and there’s always a fixed marker for where you currently are in the list, whose absolute position may change with the changes made to the list.
That way, there would be no indexing errors, although you wouldn’t be able to guarantee that the “for each” would ever terminate (i.e. might get an infinite loop). But this is a lesser evil.
August 24, 2009 at 11:55 am
The best solution could be using While loop.
While (list it not empty)
{
Delete First item
}
this will not give exception at all.
Jayvardhan Patil
http://codeforfuture.com
November 5, 2009 at 3:52 am
The reverse iteration is just what I needed. I would’ve never thought of trying that. Thanks!
November 6, 2009 at 10:08 pm
Holy cow, it’s is just appalling to see so many solutions that reek of disregard to runtime efficiency! Josh has posted a clean and solution. All solutions that include copying, converting to arrays etc are almost as bad as the sort of solutions that PHP and BASIC coders come up with.
When there are methods already present in the .NET library, *use them*. Just like Josh does. That’s what they’re there for.