Intelligent way of removing items from a List w

Question

I have the classic case of trying to remove an item from a collection while enumerating it in a loop:

List<int> myIntCollection = new List<int>();
myIntCollection.Add(42);
myIntCollection.Add(12);
myIntCollection.Add(96);
myIntCollection.Add(25);

foreach (int i in myIntCollection)
{
    if (i == 42)
        myIntCollection.Remove(96);    // The error is here.
    if (i == 25)
        myIntCollection.Remove(42);    // The error is here.
}

At the beginning of the iteration after a change takes place, an InvalidOperationException is thrown, because enumerators don’t like when the underlying collection changes.

I need to make changes to the collection while iterating. There are many patterns that can be used to avoid this, but none of them seems to have a good solution:

1. Do not delete inside this loop, instead keep a separate “Delete List”, that you process after the main loop.

   This is normally a good solution, but in my case, I need the item to be gone instantly as “waiting” till after the main loop to really delete the item changes the logic flow of my code.

2. Instead of deleting the item, simply set a flag on the item and mark it as inactive. Then add the functionality of pattern 1 to clean up the list.

   This would work for all of my needs, but it means that a lot of code will have to change in order to check the inactive flag every time an item is accessed. This is far too much administration for my liking.

3. Somehow incorporate the ideas of pattern 2 in a class that derives from List<T>. This Superlist will handle the inactive flag, the deletion of objects after the fact and also will not expose items marked as inactive to enumeration consumers. Basically, it just encapsulates all the ideas of pattern 2 (and subsequently pattern 1).

   Does a class like this exist? Does anyone have code for this? Or is there a better way?

4. I’ve been told that accessing myIntCollection.ToArray() instead of myIntCollection will solve the problem and allow me to delete inside the loop.

   This seems like a bad design pattern to me, or maybe it’s fine?

Details:

• The list will contain many items and I will be removing only some of them.

• Inside the loop, I will be doing all sorts of processes, adding, removing etc., so the solution needs to be fairly generic.

• The item that I need to delete may not be the current item in the loop. For example, I may be on item 10 of a 30 item loop and need to remove item 6 or item 26. Walking backwards through the array will no longer work because of this. ;o(

Answer 1

When you need to iterate through a list and might modify it during the loop then you are better off using a for loop:

for (int i = 0; i < myIntCollection.Count; i++)
{
    if (myIntCollection[i] == 42)
    {
        myIntCollection.Remove(i);
        i--;
    }
}

Of course you must be careful, for example I decrement i whenever an item is removed as otherwise we will skip entries (an alternative is to go backwards though the list).

If you have Linq then you should just use RemoveAll as dlev has suggested.

Answer 2

As you enumerate the list, add the one you want to KEEP to a new list. Afterward, assign the new list to the myIntCollection

List<int> myIntCollection=new List<int>();
myIntCollection.Add(42);
List<int> newCollection=new List<int>(myIntCollection.Count);

foreach(int i in myIntCollection)
{
    if (i want to delete this)
        ///
    else
        newCollection.Add(i);
}
myIntCollection = newCollection;

Answer 3

Let's add you code:

List<int> myIntCollection=new List<int>();
myIntCollection.Add(42);
myIntCollection.Add(12);
myIntCollection.Add(96);
myIntCollection.Add(25);

If you want to change the list while you're in a foreach, you must type .ToList()

foreach(int i in myIntCollection.ToList())
{
    if (i == 42)
       myIntCollection.Remove(96);
    if (i == 25)
       myIntCollection.Remove(42);
}

Answer 4

For those it may help, I wrote this Extension method to remove items matching the predicate and return the list of removed items.

    public static IList<T> RemoveAllKeepRemoved<T>(this IList<T> source, Predicate<T> predicate)
    {
        IList<T> removed = new List<T>();
        for (int i = source.Count - 1; i >= 0; i--)
        {
            T item = source[i];
            if (predicate(item))
            {
                removed.Add(item);
                source.RemoveAt(i);
            }
        }
        return removed;
    }

Answer 5

If you're interested in high performance, you can use two lists. The following minimises garbage collection, maximises memory locality and never actually removes an item from a list, which is very inefficient if it's not the last item.

private void RemoveItems()
{
    _newList.Clear();

    foreach (var item in _list)
    {
        item.Process();
        if (!item.NeedsRemoving())
            _newList.Add(item);
    }

    var swap = _list;
    _list = _newList;
    _newList = swap;
}

Answer 6

The best solution is usually to use the RemoveAll() method:

myList.RemoveAll(x => x.SomeProp == "SomeValue");

Or, if you need certain elements removed:

MyListType[] elems = new[] { elem1, elem2 };
myList.RemoveAll(x => elems.Contains(x));

This assume that your loop is solely intended for removal purposes, of course. If you do need to additional processing, then the best method is usually to use a for or while loop, since then you're not using an enumerator:

for (int i = myList.Count - 1; i >= 0; i--)
{
    // Do processing here, then...
    if (shouldRemoveCondition)
    {
        myList.RemoveAt(i);
    }
}

Going backwards ensures that you don't skip any elements.

Response to Edit:

If you're going to have seemingly arbitrary elements removed, the easiest method might be to just keep track of the elements you want to remove, and then remove them all at once after. Something like this:

List<int> toRemove = new List<int>();
foreach (var elem in myList)
{
    // Do some stuff

    // Check for removal
    if (needToRemoveAnElement)
    {
        toRemove.Add(elem);
    }
}

// Remove everything here
myList.RemoveAll(x => toRemove.Contains(x));