Why is it bad to "monkey with the loop index"?

Question

One of Steve McConnell's checklist items is that you should not monkey with the loop index (Chapter 16, page 25, Loop Indexes, PDF format).

This makes intuitive sense and is a practice I've always followed except maybe as I learned how to program back in the day.

In a recent code review I found this awkward loop and immediately flagged it as suspect.

        for ( int i=0 ; i < this.MyControl.TabPages.Count ; i++ )
        {
            this.MyControl.TabPages.Remove ( this.MyControl.TabPages[i] );
            i--;
        }

It's almost amusing since it manages to work by keeping the index at zero until all TabPages are removed.

This loop could have been written as

        while(MyControl.TabPages.Count > 0)
            MyControl.TabPages.RemoveAt(0);

And since the control was in fact written at about the same time as the loop it could even have been written as

        MyControl.TabPages.Clear();

I've since been challenged about the code-review issue and found that my articulation of why it is bad practice was not as strong as I'd have liked. I said it was harder to understand the flow of the loop and therefore harder to maintain and debug and ultimately more expensive over the lifetime of the code.

Is there a better articulation of why this is bad practice?

Answer 1

I think your articulation is great. Maybe it can be worded like so:

Since the logic can be expressed much clearer, it should.

Answer 2

Well, this adds confusion for little purpose - you could just as easily write:

while(MyControl.TabPages.Count > 0)
{
    MyControl.TabPages.Remove(MyControl.TabPages[0]);
}

or (simpler)

while(MyControl.TabPages.Count > 0)
{
    MyControl.TabPages.RemoveAt(0);
}

or (simplest)

MyControl.TabPages.Clear();

In all of the above, I don't have to squint and think about any edge-cases; it is pretty clear what happens when. If you are modifying the loop index, you can quickly make it quite hard to understand at a glance.

Answer 3

It's all about expectation.

When one uses a loopcounter, you expect that it is incremented (decremented) each iteration of the loop with the same amount.

If you mess (or monkey if you like) with the loop counter, your loop does not behave like expected. This means it is harder to understand and it increases the chance that your code is misinterpreted, and this introduces bugs. Or to (mis) quote a wise but fictional character:

complexity leads to misunderstanding

misunderstanding leads to bugs

bugs leads to the dark side.