views:

188

answers:

6

I have the following method, I wish to remove items from my collection that match the product Id. Seems fairly straight forward, but i get an exception. Basically my collection is getting out of sync. So what is the best way to remove an item from a collection.

public void RemoveOrderItem(Model.Order currentOrder, int productId)
{

    foreach (var orderItem in currentOrder.OrderItems)
    {
        if (orderItem.Product.Id == productId)
        {
            currentOrder.OrderItems.Remove(orderItem);
        }
    }
}

Exception Details: System.InvalidOperationException: Collection was modified; enumeration operation may not execute

+17  A: 

Modifying a collection inside a loop doesn’t work. To work around that, List has a few methods that allow “batch” modifications of a collection. In your case, use:

currentOrder.OrderItems.RemoveAll(x => x.Product.Id == productId)
Konrad Rudolph
thanks conrad, weirdly i can't get the lamda to work. It doesn't recognise the "x.Product.Id" part. Strange cos the following worksvar query = from x in currentOrder.OrderItems where x.Product.Id == productId select x; The collection type is ISet.
frosty
ok, i answered my own question :) I've changed this to List<t>
frosty
+3  A: 

You can't modify a collection while iterating it. Just use a normal for loop instead of a foreach loop.

ho1
+2  A: 

You can't remove an item from a collection you are iterating through, you could keep track of the orderItem, then remove it after you finish looping

BenW
+3  A: 

By looping this way you can not remove items because its in collection it keeps the track of the stored items.

Easy way to do this :

   authorsList.RemoveAll(x => x.ProductId == productId);

or

   authorsList = authorsList.Where(x => x.ProductId!= productId).ToList();
Pranay Rana
+1  A: 

As you realise you can't remove an item from a collection whilst you are looping over it. I'm sure someone will be able to provided a neater LINQ solution but the following should get you going initially:

public void RemoveOrderItem(Model.Order currentOrder, int productId)
{
    var selectedOrderItem = null;
    foreach (var orderItem in currentOrder.OrderItems)
    {
        if (orderItem.Product.Id == productId)
        {
            selectedOrderItem = orderItem;
            break;
        }
    }

    if(selectedOrderItem != null)
        currentOrder.OrderItems.Remove(selectedOrderItem);
}
openshac
Konrad Rudolph has given the LINQ solution
openshac
@openshac: Note that `List<T>.RemoveAll` is available in .NET 2.0 and is technically not LINQ, though it looks similar. That's a huge plus over LINQ for those poor souls that have to maintain Windows 2000 compatibility.
OregonGhost
A: 

"foreach" provides a "Forward-only read-only" iteration of a collection.

As a workaround, you can copy the reference to another collection and then iterate on the copied collection and remove the items from the original one.

Amby