[hasslblad]

As soon as I saw that snippet I could see what's wrong.

In C# / .Net you can't remove an element from an enumerator while you're enumerating through it. You can remove the last element however, as it's the final loop the enumerator isn't used again so it won't throw an error. The original developer probably tried to remove it forward only first, encountered an error and wrote the code to loop through it backwards, using the random tweaking technique.

What's rather depressing is that a lot of developers I've encountered use the random tweaking methodology, instead of figuring out what's really happening.

[jbri]

Actually, that's not the problem :)

The can't-modify-a-collection-while-enumerating-it issue only comes into play if you're actually using an enumerator (either directly, or as part of a foreach loop) - the code in the article uses a plain for loop along with indexing into the collection, and wouldn't run into the problem.

Rather, the primary "issue" is that without that "i--" at the end it only removes half the elements - after removing an element, all the following elements shift back one index, and so the very next element never gets removed.

[hasslblad]

Sorry, I should have been clearer. I was getting flashbacks to when I saw a similar problem (except in a for each loop), that's what set the alarm bells ringing in my head.

When I see nasty code like that, I tend to stop parsing it fully and sniff out the intent. I think it's a form of bad code blindness (like banner ad blindness) my brain is protecting me from all the bad code I've seen. If I fully parsed all the really bad code properly I’d become a dribbling wreck. :) So I tend to look at it at a higher level instead to stay sane.

[masklinn]

1. No iterator is being used here, so the iterator coherence check does not come into play

2. List elements are removed from the front, the code is essentially a complicated version of:

    while (0 < this.MyControl.TabPages.Count) {
        this.MyControl.TabPages.Remove(0);
    }

[JoeAltmaier]

I've also found it depressing that you can't remove an element fron an enumerator while you're enumerating through it

Why? I like to think library developers have my back. What's the point in an iterator, it its broken on the 1st thing I try? (Ok the 2nd thing; 1st I code a search through the list, then the teardown)

Iterators are also pretty much broken when 1) inserting into a list, 2) merging a list into a list, 3) deleting an item from a list.

Ok, they're pretty much broken for absolutely everything BUT searching lists.

I remember the profound disappointment I felt when Java 1 came out, and its iterators were this same lame junk.

So I never use them. They are born to create bugs like this one.

[roel_v]

Yeah well except that that's not even related to what's happening here. Good thing though that you spotted it immediately and that you're smarter than all the developers you encounter ^_^

[hasslblad]

Well, when working on a code base, the first thing I think is who wrote this and when. When they wrote this how experienced was the developer? What's there coding style or was it written after coding standards were introduced? I've had this misfortune of inheriting a lot of bad legacy code in the past and when you see a chunk of bad code, you can either spend a good chunk of time figuring what it's really doing or less time figuring out the intent. Depending on who wrote the code, what it does, and various other factors you can decide which path to take.

With this code snippet it's obvious that the intent is to remove all the elements. So it can be fixed with a .Clear();

From a higher level, you can see that the snippet smells bad, and will need some attention.

I didn't say I'm smarter than all the developers I encounter. I've just encountered a lot of bad developers in my time.