Avoid Collection has been modified error

Question

Issue:

I have the following code:

foreach(var ItemA in GenericListInstanceB)
{
    ItemA.MethodThatCouldRemoveAnyItemInGenericListInstanceB();
}

Obviously i get an error.

What i want to do is change GenericListInstanceB to a class i create, that keeps track of items it should remove. Then, when the loop is finished, it removes them all. Same with adding items.

This is fine, i can create a class that has an internal list, and a list of items to add and items to remove. Then another method called AddRemovePendingItems that actually 'updates' the list (removes and adds items accordingly). The tricky part is getting that method to be called automatically.

Ideas: Maybe looking at when GetEnumerator is called? Maybe doing something clever with IDisposable?

Question: How do i know when the for loop exits and we've finished iterating over my collection class so i can call my AddRemovePendingItems method?, particularly if the loop break's early?

Note I want to avoid any unnecessary copying as i want to eliminate/minimise garbage collection so i can't just iterate over a copy/make a new copy or anything like that.

Another Idea: copy the items you want to keep to another generic list — Jodrell, Jun 10 '11 at 10:45
@Jodrell, unfortunately that's not an option for me as this should run on the Xbox, where i'm trying to remove any unnecessary copying. — George Duckett, Jun 10 '11 at 10:49
Why does `ItemA` have a reference to `GenericListInstanceB`? — leppie, Jun 10 '11 at 10:51
@George Duckett: you will have to use a for loop then, and modify the index when deleting an item. — leppie, Jun 10 '11 at 10:52
@George - You would be copying references and not objects. I think your reluctance to do so is misguided. — Ritch Melton, Jun 10 '11 at 10:52
@Leppie, I'm not sure that's the only option. Plus, i'm not sure it is an option since i don't know which items might be added or removed. — George Duckett, Jun 10 '11 at 10:53
@Ritch, The old list would need GCing though. I'd also need to manage what items i want to keep somewhere, not sure how to do that since the list is a field accessed by many methods in the class. Sorry if i'm not understanding the suggested solution or am not explaining myself very well. — George Duckett, Jun 10 '11 at 10:58
I dont think it can be done. `Enumerator` does not expose any functionality you could 'plug' into to know when it has finished enumerating the collection. I'd just go with a regular loop from last to first item deleting as I go along. I'm not sure why you are reluctant to do it that way. — InBetween, Jun 10 '11 at 11:02
@Jodrell, Not sure how [tag:linq] would help here. I want to avoid creating a new list. — George Duckett, Jun 10 '11 at 11:02
@InBetween, see responses to @Prakash's answer as to why i couldn't do that. I'm thinking i might be able to do something clever with IDisposable, then use a foreach loop wrapped in a using statement. Ideally doing something within the GetEnumerator to enforce the foreach being within the using block. — George Duckett, Jun 10 '11 at 11:14
@George - Emulating RAII style semantics with using() isn't that unsual though. — Ritch Melton, Jun 10 '11 at 11:15
@Ritch, not officially, but i do write code that does the job, only doing something clever if needed (which i think in this case it would greatly reduce errors (forgetting to call an AddRemovePendingItems method). If someone know's how to do what i'm asking i'd be greatfull for an answer, even if it had a precursor of "use at your own risk" — George Duckett, Jun 10 '11 at 11:17
@Ritch, that looks like what i'm after this link (http://www.codeproject.com/KB/cs/raiihelper.aspx) looks interesting. — George Duckett, Jun 10 '11 at 11:21
My Answer now does what you want without any magic (I think) — Jodrell, Jun 10 '11 at 12:34
Also see http://stackoverflow.com/questions/759966/c-sharp-what-is-the-best-way-to-modify-a-list-in-a-foreach-loop?lq=1 — nawfal, Jun 05 '13 at 17:44

score 3 · Accepted Answer · answered Jun 10 '11 at 12:39

3

You mentioned IDisposable, which offers one way you could implement this:

public class GenericList<T> : IList<T>
{
    private class CleanupEnumerator<T> : IEnumerator<T>
    {
        private readonly GenericList<T> source;

        public CleanupEnumerator<T>(GenericList<T> source)
        {
            this.source = source;
        }

        public void Dispose()
        {
            source.RemovePendingDeletes();
        }

        /* Other IEnumerator methods here */
    }

    public IEnumerator<T> GetEnumerator()
    {
        return new CleanupEnumerator(this);
    }

    /* Other IList methods here */
}

This will guarantee that any time your collection is enumerated using foreach, the RemovePendingDeletes() function will get called when the enumerator is disposed. Be warned, however, that this could get ugly if you ever call GetEnumerator() directly and forget to dispose the enumerator.

answered Jun 10 '11 at 12:39

JSBձոգչ

40,684
18
101
169

This looks like what i'm after. Would the Dispose method definately get called as soon as control leaves the forloop block (whether it be a `break` or a normal exit? – George Duckett Jun 10 '11 at 12:56
@George, `foreach` will guarantee that the Dispose method gets called. `foreach` is basically syntactic sugar for `using (var e = o.GetEnumerator()) { while (e.MoveNext()) { ... } }`. – JSBձոգչ Jun 10 '11 at 13:00
But even with this solution the pending deletes need to be stored somewhere. This somewhere will probably be some kind of collection that will consume memory and produce GC pressure. In my opinion this does not fulfill the OPs requirement of "_I want to avoid any unnecessary copying as i want to eliminate/minimise garbage collection so i can't just iterate over a copy/make a new copy or anything like that._" – Florian Greinacher Jun 10 '11 at 13:10
Nevertheless without this requirement the solution would be a pretty elegant solution to the problem! – Florian Greinacher Jun 10 '11 at 13:10
@Florain. That's true, however if the itemstoremove / itemstoadd lists could be persistent (preventing a nested foreach, i think, but that's ok) then there shouldn't be any GC pressure. – George Duckett Jun 10 '11 at 13:23
@George, @Florian, you can also avoid this error just by implementing your own collection that doesn't care if you delete while iterating. This will require some cleverness with your implementation of the collection and the enumerator, but I've done it myself when I needed it. – JSBձոգչ Jun 10 '11 at 13:26
@JSBangs I'd be very interested in seeing the implementation of such a collection. It sounds like a good solution to my problem (although i recognise, not quite what i asked). – George Duckett Jun 10 '11 at 13:30
@George, see http://code.google.com/p/phonix/source/browse/trunk/Core/Word.cs and http://code.google.com/p/phonix/source/browse/trunk/Core/WordNode.cs. It's basically a linked list where every node has a `Delete()` and `Insert()` method. The nodes handle their own bookkeeping in the case of deletion/insertion, and the parent list does a bog-standard linked list traversal in its `GetEnumerator()`. If you can't use a linked list, you can implement something similar backed by a flat array, though it's a little bit more work – JSBձոգչ Jun 10 '11 at 13:47

Jodrell · Answer 2 · 2011-06-10T12:36:10.040

0

How about this. I'm assuming the MethodThatCouldRemoveAnyItemInGenericListInstanceB function removes one or no items from anywhere in the list, no items will be added.

bool finished = false;
int i = 0;
while (!finished)
{
    var itemA = genericListInstanceB[i];
    itemA.MethodThatCouldRemoveAnyItemInGenericListInstanceB();
    if (genericListInstanceB[i] == itemA)
        i++;
        // All other outcomes result in us leaving i alone
    finished = (i > (genericListInstanceB.Count - 1));
}

Dangerous, old fashioned and "smelly" but, it does what you want without having to have a specialised list or some magic to trap the disposal of the list.

edited Jun 10 '11 at 12:36

answered Jun 10 '11 at 11:42

Jodrell

34,946
5
87
124

Thanks for all the effort, but i think there's still corder cases (maybe i should ahve been more clear). Things like if one item as added and one is removed, or if 2 items are added etc. – George Duckett Jun 10 '11 at 12:54
Yep, this obviously won't work for that. The only idea I have in that scenario is a iterative function that works out where to go next. This problem can't be solved without some extra state. Whether that is internal to a specialised list, or perhaps a hashtable to detect changes between operations. Do the additions have to be processed too? – Jodrell Jun 10 '11 at 13:21

Avoid Collection has been modified error

2 Answers2

Linked