.NET - Remove from a List<T> within a 'foreach' loop

Viewed 60872

I have code that I want to look like this:

List<Type> Os;

...

foreach (Type o in Os)
    if (o.cond)
        return;  // Quitting early is important for my case!
    else
        Os.Remove(o);

... // Other code

This doesn't work, because you cannot remove from the list when you are inside a foreach loop over that list:

Is there a common way to solve the problem?

I can switch to a different type if needed.

Option 2:

List<Type> Os;

...

while (Os.Count != 0)
     if (Os[0].cond)
         return;
     else
         Os.RemoveAt(0);

... // Other code

Ugly, but it should work.

17 Answers

You can iterate through the list backwards:

for (int i = myList.Count - 1; i >= 0; i--)
{
    if (whatever) myList.RemoveAt(i);
}

In response to your comment about wanting to quit when you find an item that you're NOT removing, then just using a while loop would be the best solution.

You should never remove anything from a collection you are iterating over while inside of a foreach loop. It's basically like sawing the branch you are sitting on.

Use your while alternative. It is the way to go.

Do you really need to do this within a foreach loop?

This will achieve the same results as your examples, ie, remove all items from the list up until the first item that matches the condition (or remove all items if none of them match the condition).

int index = Os.FindIndex(x => x.cond);

if (index > 0)
    Os.RemoveRange(0, index);
else if (index == -1)
    Os.Clear();

I am a Java programmer, but something like this works:

List<Type> Os;
List<Type> Temp;
...
foreach (Type o in Os)
    if (o.cond)
        Temp.add(o);
Os.removeAll(Temp);  

I just had that problem with my analysis library. I tried this:

for (int i = 0; i < list.Count; i++)
{                
   if (/*condition*/)
   {
       list.RemoveAt(i);
       i--;
   }
}

It's pretty simple but I haven't thought of any breaking point.

 Os.RemoveAll(delegate(int x) { return /// });

I'd try finding the index of first item that does not satisfy the predicate and do RemoveRange(0, index) on it. If nothing else, there should be less Remove calls.

Update: Added for completeness

As several have answered, you shouldn't modify a collection while iterating it with GetEnumerator() (example foreach). The framework prevent you from doing this by throwing an exception. The generic colution to this is to iterate "manually" with for (see other answers). Be careful with your index so you don't skip items or re-evaluate the same one twice (by using i-- or iterating backward).

However, for your specific case, we can optimize the remove operation(s)... original answer below.


If what you want is to remove all items until one meets a given condition (that's what your code does), you can do this:

bool exitCondition;

while(list.Count > 0 && !(exitCondition = list[0].Condition))
   list.RemoveAt(0);

Or if you want to use a single remove operation:

SomeType exitCondition;
int index = list.FindIndex(i => i.Condition);

if(index < 0)
    list.Clear();
else
{
    exitCondition = list[0].State;
    list.RemoveRange(0, count);
}

Note: since I'm assuming that item.Condition is bool, I'm using item.State to save the exit condition.

Update: added bounds checking and saving exit condition to both examples

If you know your list isn't very large you can use

foreach (Type o in new List<Type>(Os))
    ....

which will create a temporary duplicate of the list. Your remove() call will then not be interfering with the iterator.

Look at Enumerable.SkipWhile()

Enumerable.SkipWhile( x => condition).ToList()

Generally not mutating a list, makes live a lot easier. :)

Anzurio's solution is probably the most straightforward, but here's another clean one, if you don't mind adding a bunch of interfaces/classes to your utilities library.

You can write it like this

List<Type> Os;
...
var en = Os.GetRemovableEnumerator();
while (en.MoveNext())
{
    if (en.Current.Cond)
        en.Remove();
}

Put the following infrastructure, inspired by Java's Iterator<T>.remove, into your utility library:

static class Extensions
{
    public static IRemovableEnumerator<T> GetRemovableEnumerator<T>(this IList<T> l)
    {
        return new ListRemovableEnumerator<T>(l);
    }
}

interface IRemovableEnumerator<T> : IEnumerator<T>
{
    void Remove();
}

class ListRemovableEnumerator<T> : IRemovableEnumerator<T>
{
    private readonly IList<T> _list;
    private int _count;
    private int _index;
    public ListRemovableEnumerator(IList<T> list)
    {
        _list = list;
        _count = list.Count;
        _index = -1;
    }

    private void ThrowOnModification()
    {
        if (_list.Count != _count)
            throw new InvalidOperationException("List was modified after creation of enumerator");
    }
    public void Dispose()
    {
    }

    public bool MoveNext()
    {
        ThrowOnModification();
        if (_index + 1 == _count)
            return false;
        _index++;
        return true;
    }

    public void Reset()
    {
        ThrowOnModification();
        _index = -1;
    }

    object IEnumerator.Current
    {
        get { return Current; }
    }

    public T Current
    {
        get { return _list[_index]; }
    }

    public void Remove()
    {
        ThrowOnModification();
        _list.RemoveAt(_index);
        _index--;
        _count--;
    }
}
Related