Collection was modified; enumeration may not execute error when removing a ListItem from a LIstBox

Question

I have two ListBoxes, lstAvailableColors and lstSelectedColors. Between each listbox are two buttons, Add and Remove. When a color or colors is selected in lstAvailableColors and the Add button is clicked, I want to remove them from lstAvailableColors and display them in lstSelectedColors. Also, if colors are selected in lstSelectedColors and the Remove button is clicked, I want to remove the colors from lstSelectedColors and add them back to lstAvailableColors. When I do this, I get the following error when it removes the item:

Collection was modified; enumeration operation may not execute.

Here is the code for the Add Button and the Remove Button:

Add:

protected void btnAdd_Click(object sender, EventArgs e)
{
    foreach (ListItem item in lstAvailableColors.Items)
    {
        if (item.Selected)
        {
            lstSelectedColors.Items.Add(item);
            lstAvailableColors.Items.Remove(item);
        }
    }
}

Remove:

protected void btnRemove_Click(object sender, EventArgs e)
{
    foreach (ListItem item in lstSelectedColors.Items)
    {
        if (item.Selected)
        {
            lstAvailableColors.Items.Add(item);
            lstSelectedColors.Items.Remove(item);
        }
    }
}

Solution 1

It's not possible to modify a collection while you're enumerating it in .Net. You need to separate out your enumeration and remove code into different blocks. Here is a quick sample on how to do that in without LINQ

protected void btnAdd_Click(object sender, EventArgs e)
{
    var selected = new List<ListItem>();
    foreach (ListItem item in lstAvailableColors.Items)
    {
        if (item.Selected)
        {
            selected.Add(item);
            lstSelectedColors.Items.Add(item);
        }
    }
    foreach (ListItem item in selected)
    {
        lstAvailableColors.Items.Remove(item);
    }
}

And here's a more concise version using LINQ

var selected = lstAvailableColors.Cast<ListItem>().Where(i => i.Selected).ToList();
selected.ForEach( x => { lstSelectedColors.Items.Add(x); });
selected.ForEach( x => { lstAvailableColors.Items.Remove(x);});

EDIT

The LINQ version works in two parts. The first part is the first line which finds the currently selected items and stores the value in a List<ListItem>. It's very important that the line contain the .ToList() call because that forces the query to execute immediately vs. being delayed executed.

The next two lines iterate through each value which is selected and remove or add it to the appropriate list. Because the selected list is already stored we are no longer enumerating the collection when we modify it.

Solution 2

You cannot modify an collection while you are using an Enumerator for this collection, what the for each statement does.

You have to loop over the data with a normal for loop and then you can modify the collection, but you must be careful to correctly update the current index if you insert or remove elements. If you just add or remove elements and don't insert some, iterating from the last element to the first will do.

protected void btnAdd_Click(object sender, EventArgs e)
{
    for (Int32 i = lstAvailableColors.Items.Count; i >= 0; i--)
    {
        ListItem item = lstAvailableColors.Items[i];

        if (item.Selected)
        {
            lstSelectedColors.Items.Add(item);
            lstAvailableColors.Items.Remove(item);
        }
    }
}

Solution 3

You cannot modify a collection you are iterating on. In general, a good solution for this type of problem is to create an empty collection, and in your iterator, copy over all of the elements you do NOT want to remove; after the iteration is complete, replace the original collection with your new collection.

Solution 4

As the other answer mentioned, you can't remove items until you've completed the iteration. So perhaps something like this will be cleanest for you:

var itemsToRemove =
lstAvailableColors.Items.Cast<ListItem>().Where(i => i.IsSelected).ToArray();

foreach(ListItem item in itemsToRemove) lstAvailableColors.Remove(item);

Solution 5

You can't modify a collection while you're iterating over it. Either iterate over a copy or use for, iterate in reverse and remove as you go down.

View more solutions

Comments

[Shea]

for (int i=lstSelectedColors.Items.Count-1; i >= 0; i--) { if ( lstSelectedColors.Items[i].Selected] ) { lstSelectedColors.Items.Remove(lstSelectedColors.Items[i]); } }

[Vishal_Kotecha]

Can you explain the LINQ version?

[plankalkul]

You contradict yourself; you say you cannot modify a collection while you are iterating over it (no direction specified), but you immediately say that the solution is to iterate in reverse. It's just generally a bad idea to modify a collection you're iterating over, forward or reverse.

[Shea]

I was using iterate too loosely. Foreach enumerates over a collection. You can't modify it while enumerating over the collection. But you can modify it by using a for loop and starting at the end of the collection.

[Lucas]

"with LINQ" should be "without LINQ" in the first example ;)

[Shea]

agreed, though I'm curious - why do you say it's bad form? What's bad about it?

[Charles Bretana]

because even if you iterate in reverse, there's nothing to stop you from modifying items in the enumeration BEFORE the one you are currently on, and then you're hosed. You can iterate forward and [carefully] modify items you haven't gotten to yet as well, but its just generally way too easy to screw this up, so it better not to dio it at all, as a matter of good practice

[apereira]

Thank you for your solution but i need to ask if this is possible with one of the listboxes being able to select more than one value.