RemoveAt in foreach { ... }

[Hyun-jik Bae]

Is there any efficient way doing Dictionary<>.RemoveAt in foreach { ... }
routine?
For example, is this example safe or is there any recommended way?

foreach(KeyValuePair<int,int> a in mymap)
{
if(a.Value!=GoodValue)
{
mymap.RemoveAt(a.Key); // and continues the iteration
}
}

Please reply. Thanks in advance.

Hyun-jik Bae

 

[Jon Slaughter]

Is there any efficient way doing Dictionary<>.RemoveAt in foreach { ... }
routine?
For example, is this example safe or is there any recommended way?

foreach(KeyValuePair<int,int> a in mymap)
{
if(a.Value!=GoodValue)
{
mymap.RemoveAt(a.Key); // and continues the iteration
}
}

I don't remember exactly but I was reading something about this the other
day and I believe its not safe but there is a good way to fix it.

I'm not sure if it involes working on a temp copy or not though.

i.e., you could easily get around it by creating a shallow copy of mymap and
then working on that temp in the inner loop.


Sorry but thats the best I could do(I though I'd mention it just incase you
don't get to many replies).

Jon

 

[=?ISO-8859-1?Q?G=F6ran_Andersson?=]

You can't change the collection that you are looping through. Changing
the collection makes the enumerator invalid, and it will throw an
exception when you try to continue the loop.

One way to do it is to add the keys that you want to remove to a list,
then when you have looped through the dictionary, you loop throught the
list to remove the items from the dictionary.

 

[Brian Gideon]

Hyun-jik Bae,

Which collection are you using? Dictionary does not have a RemoveAt
method.

As others have said removing an item will invalidate the enumerator.
It's better to figure out what you want to remove in one loop and then
actually remove in another.

Brian

 

[=?ISO-8859-1?Q?G=F6ran_Andersson?=]

Which collection are you using? Dictionary does not have a RemoveAt
method.

As he was feeding the key into the method, I assume that he means the
Remove method, that takes a key as argument. A RemoveAt method would
take an index instead of a key.

 

[Cor Ligthert [MVP]]

Hyun-jik,

Although that there are possibilities, I never will use RemoveAt in a
foreach loop.

I will alway use to RemoveAt a For index with a bottom up loop.

Cor

 

[Gaurav Vaish \(www.EdujiniOnline.com\)]

Although that there are possibilities, I never will use RemoveAt in a

foreach loop.

I don't think there are any possibilities.
The enumerator would immediately become invalid....


Correct way:

1. Iterate through all entries.
2. Grab all those required in another list.
3. Remove all these "collected" (grabbed) entries from the original
list/collection.

The only way.


--
Happy Hacking,
Gaurav Vaish | www.mastergaurav.com
www.edujinionline.com
http://eduzine.edujinionline.com
-----------------------------------------

 

[Guest]

enumerator is invalidated. use:

while (dict.Count > 0)
dict.RemoveAt(0);

 

[Cor Ligthert [MVP]]

Correct way:

1. Iterate through all entries.
2. Grab all those required in another list.
3. Remove all these "collected" (grabbed) entries from the original
list/collection.

The only way.

No bottom up is easier,

For (int i = mycollection.count - 1;i > -1;i--)
{ if (whatever) removeAt(i);}

Cor

 

[Guest]

or
while (dict.Count > 0)
dict.RemoveAt(0);

 

[Jon Skeet [C# MVP]]

or
while (dict.Count > 0)
dict.RemoveAt(0);

That only works if you're trying to clear the whole thing though - in
which case there are usually much simpler ways of doing it.

 

[Peter Duniho]

No bottom up is easier,

For (int i = mycollection.count - 1;i > -1;i--)
{ if (whatever) removeAt(i);}

Just to be ornery, I'll point out that I like top down better:

int i = 0
while (i < mycollection.Count)
{
if (!whatever)
{
i++;
continue;
}
mycollection.RemoveAt(i);
}




(I actually do like that better, because the code is IMHO more readable; too
many "minuses" and "offsets by one" in the bottom up version for my tastes)

Pete

 

[Hyun-jik Bae]

Sorry, that's my mistake. I should have shown Remove(), not RemoveAt().

Hyun-jik Bae

 

[Brian Gideon]

Hyun-jik Bae,

No need to be sorry. I was just wanting clarification. Obviously, all
of the responses regarding RemoveAt are irrelevant now. I think your
best option is to use the two loop method; one to figure out what you
want to remove and the other to actually remove them.

Brian

 

[Michael D. Ober]

The top down approach has another major benefit - it doesn't invalidate the
indexes of elements you haven't examed yet for removal.

Mike Ober.

 

[Cor Ligthert [MVP]]

Michael,

Feel free to use even 100 loops if you want. I think that one short loop
bottom up through the index is never to beat. Probably nobody has tested it
but are just telling that their long time taking approaches are the best.

This method is however not from me. Armin Zingler came with it I thought
about 3 years ago in the Visual Basic newsgroup, where all what is showed
now in this newsgroup was showed before that.

Now nobody active in the Visual Basic newsgroup will think on other than
bottom up methods for these problems, while they did before this top down,
with all those work around methods showed here.

Cor

 

[Peter Duniho]

Feel free to use even 100 loops if you want. I think that one short loop
bottom up through the index is never to beat.

"Never to beat"? In what respect? In programming, there are far too many
competing goals for anyone to say one solution is the best one 100% of the
time.

I think it unlikely that if you are iterating through an entire indexed
collection and removing some subset of that collection, you'll see a
significant difference between various means of iteration, in the most
common cases.

If you're dealing with an array, and each removal requires that the
remaining elements in the array be copied down, then yes...starting at the
end will result in a slightly better performance. However, unless you're
deleting most of the members of the array, you won't notice this difference.
The total data moved will only be a little less that way, and it ignores the
benefit of code clarity.

If performance is an issue, then the right way is to simply copy the
references to be preserved to a new array, and then just delete the entire
original. This method will perform WAY better than either a top-down or
bottom-up in-place removal. If performance is not an issue, then you can't
use performance as the sole measure of which method is better.

If you're dealing with a linked list, then removal by index is going to
perform poorly whichever end you start at, and there will be better
performance starting at the beginning (top-down) than the end (bottom-up).
A better way than using an index from either end would be to iterate through
the list by reference, so that removal operations have constant order.

[...]
This method is however not from me. Armin Zingler came with it I thought
about 3 years ago in the Visual Basic newsgroup, where all what is showed
now in this newsgroup was showed before that.

I hate to tell you this, but the question of removing elements from an
indexed array is so old, it's unlikely anyone could say who "came with it".
Neither your method nor mine is from either of us, nor originally from Armin
Zingler (whoever he is) and that fact goes without saying. These concepts
predate .NET, Windows, and the PC.

Now nobody active in the Visual Basic newsgroup will think on other than
bottom up methods for these problems, while they did before this top down,
with all those work around methods showed here.

What work-around methods? The correct top-down method is equivalent to the
bottom-up method, and carries with it no greater degree of "work-around"
than the bottom-up method.

If "nobody active in the Visual Basic newsgroup will think on other than
bottom up methods for these problems", then that's because nobody in that
newsgroup understands that in programming, one size does not fit all. In
reality, I doubt your assertion is true, but if it is, it's only proof of
one-dimensional thinking, rather than the validity of your claim that one
method is better than the other.

Pete

 

[Martin Z]

Just to be ornery, I'll point out that I like top down better:

int i = 0
while (i < mycollection.Count)
{
if (!whatever)
{
i++;
continue;
}
mycollection.RemoveAt(i);
}




(I actually do like that better, because the code is IMHO more readable; too
many "minuses" and "offsets by one" in the bottom up version for my tastes)

Pete

Top-down or bottom-up, either way you have to worry. Top-down can
easily produce a hairy starting fencepost error because it's less
commonly used. Bottom-up can produce nastier logic errors on removal.

Imho, the DictionaryEntry<> should provide a "Remove" method that
abstracts away the hairiness of iteration/removal from a dict.
Likewise for lists, exposing a ListEntries enumerator that iterates
across ListEntry<> objects that provide a similar feature.