Best practice for iterating over a collection and modifying it?

Question

I am trying to iterate over a collection of items, perform an action and then remove the item at the current index.

The way I have this implemented currently is like so:

foreach (CormantRadPane pane in GetPanes().ToList())
{
    pane.Clear();
    StateManager.Remove(pane);
    LayoutManager.Instance.RegisteredPanes.Remove(pane);
    Items.Remove(pane);
}

By calling ToList I create a copy of the collection, but maintain a reference to each object in the first collection. This allows me to iterate over the collection returns from GetPanes without 'technically' modifying the collection.

This is clearly prone to some seriously hard to track down bugs. What is the standard way of performing such logic in a clean manner, but that also is more clear about the intriciaces of what is occurring?

I did some looking around and saw such things as using a for loop and going backwards through the list, but that just seems really bulky. My feelings are about the same with keeping a second list of items "to remove" and then iterating through that list after the first loop has finished, removing each object found in the second list.

How do you handle it? Thanks.

possible duplicate of [How to modify or delete items from an enumerable collection while iterating through it in C#](http://stackoverflow.com/questions/308466/how-to-modify-or-delete-items-from-an-enumerable-collection-while-iterating-thro) — Randy Levy, Nov 01 '11 at 20:45
@Tuzo I saw this post, and mentioned the solution provided in my thoughts. The first comment of the accepted solution said the solution was flawed, then it was fixed, then people had several other thoughts on better ways to do it. IMO there is still a fair bit of discussion to be had. — Sean Anderson, Nov 01 '11 at 20:49

score 5 · Accepted Answer · edited Nov 01 '11 at 21:22

5

Instead of current approach you can create a variable and use it, and I suggest use for loop for pane removal, trick is just iterate it in reverse:

var panes = GetPanes();
int count = panes.Count;
for(int i= count - 1; i>=0; i--)
{
       pane = panes[i];
       pane.Clear();
       StateManager.Remove(pane);
       LayoutManager.Instance.RegisteredPanes.Remove(pane);
       //Items.Remove(pane);
       Items.RemoveAt(i);
}

calling Items.Remove(pane); takes O(n) (if is list) but in the case of using Items.RemoveAt(i); it takes O(1), So your current approach takes O(n^2) but if you can call RemoveAt(index) (you have some list and they are sorted in same way) you can handle it in O(n).

edited Nov 01 '11 at 21:22

Sean Anderson

27,963
30
126
237

answered Nov 01 '11 at 20:44

Saeed Amiri

22,252
5
45
83

Accessing the items by index is the way to go here – Glenn Ferrie Nov 01 '11 at 20:50
I don't believe my current code creates a new list every time it iterates through the collection. If it does, then I have certainly learned something, but my understanding was that it creates a copy of the collection but holds references to the objects in the first collection. How could I test this? – Sean Anderson Nov 01 '11 at 20:51
@Sean Anderson you are right I edited, but this way is common way to handling this, as I see similar question there was an ugly way which accepted as answer and there a was a normal way like this, you can do many thing, but every developer knows what's the meaning of this code, and maintenance is easier. – Saeed Amiri Nov 01 '11 at 20:58
@SaeedAmiri Yeah, I definitely agree with you. I think I am just hesitant because -I- am not familiar with the implementation. I spoke with my coworker and he said that iterating backwards is common practice. Just surprised to be in such a mature language and still iterating backwards over a for loop :) – Sean Anderson Nov 01 '11 at 21:02
Very insightful. Now you've got me looking through all my code for places I use Remove instead of RemoveAt. :) – Sean Anderson Nov 01 '11 at 21:09

score 0 · Answer 2 · answered Nov 01 '11 at 20:48

0

You can use the linq list extension such as:

List<Item> items = GetItems(); items.ForEach(i => {i.DoStuff();i.DoStuff2(););}

At least that is what I typically do.

answered Nov 01 '11 at 20:48

Etch

3,044
25
31

I believe that is what my code currently does. I call ToList() in the declaration of my foreach loop, which should be the same as your items object. Then, I iterate over the items object. Just a bit more compact. – Sean Anderson Nov 01 '11 at 20:55

score 0 · Answer 3 · answered Nov 01 '11 at 20:58

What you are referring to is a Robust Iterator but I do not believe the iterator provided in both C# and Java are robust.

If all you want is to iterator through List robustly, you can implement your own Robust Iterator by keeping track of the index of the list. This may not work for collections that have less defined ordering and for some non-random access iterables like database result sets.

Perhaps another answerer can help you find a way to not require a robust iterator in your specific case.

Best practice for iterating over a collection and modifying it?

3 Answers3