Can't add/remove items from a collection while foreach is iterating over it

Question

If I make my own implementation of IEnumerator interface, then I am able ( inside foreach statement )to add or remove items from a albumsList without generating an exception.But if foreach statement uses IEnumerator supplied by albumsList, then trying to add/delete ( inside the foreach )items from albumsList will result in exception:

class Program
{
    static void Main(string[] args)
    {

        string[] rockAlbums = { "rock", "roll", "rain dogs" };
        ArrayList albumsList = new ArrayList(rockAlbums);
        AlbumsCollection ac = new AlbumsCollection(albumsList);
        foreach (string item in ac)
        {
            Console.WriteLine(item);
            albumsList.Remove(item);  //works

        }

        foreach (string item in albumsList)
        {
            albumsList.Remove(item); //exception
        }



    }

    class MyEnumerator : IEnumerator
    {
        ArrayList table;
        int _current = -1;

        public Object Current
        {
            get
            {
                return table[_current];
            }
        }

        public bool MoveNext()
        {
            if (_current + 1 < table.Count)
            {
                _current++;
                return true;
            }
            else
                return false;
        }

        public void Reset()
        {
            _current = -1;
        }

        public MyEnumerator(ArrayList albums)
        {
            this.table = albums;
        }

    }

    class AlbumsCollection : IEnumerable
    {
        public ArrayList albums;

        public IEnumerator GetEnumerator()
        {
            return new MyEnumerator(this.albums);
        }

        public AlbumsCollection(ArrayList albums)
        {
            this.albums = albums;
        }
    }

}

a) I assume code that throws exception ( when using IEnumerator implementation A supplied by albumsList ) is located inside A?

b) If I want to be able to add/remove items from a collection ( while foreach is iterating over it), will I always need to provide my own implementation of IEnumerator interface, or can albumsList be set to allow adding/removing items?

thank you

Answer 1

From the MSDN documentation for INotifyCollectionChanged:

You can enumerate over any collection that implements the IEnumerable interface. However, to set up dynamic bindings so that insertions or deletions in the collection update the UI automatically, the collection must implement the INotifyCollectionChanged interface. This interface exposes the CollectionChanged event that must be raised whenever the underlying collection changes.

WPF provides the ObservableCollection<(Of <(T>)>) class, which is a built-in implementation of a data collection that exposes the INotifyCollectionChanged interface. For an example, see How to: Create and Bind to an ObservableCollection.

The individual data objects within the collection must satisfy the requirements described in the Binding Sources Overview.

Before implementing your own collection, consider using ObservableCollection<(Of <(T>)>) or one of the existing collection classes, such as List<(Of <(T>)>), Collection<(Of <(T>)>), and BindingList<(Of <(T>)>), among many others.

If you have an advanced scenario and want to implement your own collection, consider using IList, which provides a non-generic collection of objects that can be individually accessed by index and provides the best performance.

Sounds to me that the problem is in the Collection itself, and not its Enumerator.

Answer 2

Easiest way is to either reverse through the items like for(int i = items.Count; i >=0; i--), or loop once, gather all the items to remove in a list, then loop through the items to remove, removing them from the original list.

Answer 3

Generally it's discouraged to design collection classes that allow you to modify the collection while enumerating, unless your intention is to design something thread-safe specifically so that this is possible (e.g., adding from one thread while enumerating from another).

The reasons are myriad. Here's one.

Your MyEnumerator class works by incrementing an internal counter. Its Current property exposes the value at the given index in an ArrayList. What this means is that enumerating over the collection and removing "each" item will actually not work as expected (i.e., it won't remove every item in the list).

Consider this possibility:

The code you posted will actually do this:

1. You start by incrementing your index to 0, which gives you a Current of "rock." You remove "rock."
2. Now the collection has ["roll", "rain dogs"] and you increment your index to 1, making Current equal to "rain dogs" (NOT "roll"). Next, you remove "rain dogs."
3. Now the collection has ["roll"], and you increment your index to 2 (which is > Count); so your enumerator thinks it's finished.

There are other reasons this is a problematic implementation, though. For instance someone using your code might not understand how your enumerator works (nor should they -- the implementation should really not matter), and therefore not realize that the cost of calling Remove within a foreach block incurs the penalty of IndexOf -- i.e., a linear search -- on every iteration (see the MSDN documentation on ArrayList.Remove to verify this).

Basically, what I'm getting at is: you don't want to be able to remove items from within a foreach loop (again, unless you're designing something thread-safe... maybe).

OK, so what is the alternative? Here are a few points to get you started:

1. Don't design your collection to allow -- let alone expect -- modification within an enumeration. It leads to curious behavior such as the example I provided above.
2. Instead, if you want to provide bulk removal capabilities, consider methods such as Clear (to remove all items) or RemoveAll (to remove items matching a specified filter).
3. These bulk-removal methods can be implemented fairly easily. ArrayList already has a Clear method, as do most of the collection classes you might use in .NET. Otherwise, if your internal collection is indexed, a common method to remove multiple items is by enumerating from the top index using a for loop and calling RemoveAt on indices where removal is desired (notice this fixes two problems at once: by going backwards from the top, you ensure accessing each item in the collection; moreover, by using RemoveAt instead of Remove, you avoid the penalty of repeated linear searches).
4. As an added note, I would strongly encourage steering clear of non-generic collections such as ArrayList to begin with. Go with strongly typed, generic counterparts such as List(Of Album) instead (assuming you had an Album class -- otherwise, List(Of String) is still more typesafe than ArrayList).