为什么“带有循环索引的猴子”不好？

Question

Steve McConnell 的清单项目之一是 you should not monkey with the loop index（第 16 章，第 25 页，循环索引，PDF 格式）。

这很直观，并且是我一直遵循的一种做法，除非我当时学会了如何编程。

在最近的一次代码审查中，我发现了这个尴尬的循环，并立即将其标记为可疑。

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

这几乎很有趣，因为它设法将索引保持为零，直到所有 TabPage 都被删除。

这个循环可以写成

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

而且由于实际上控件与循环几乎同时编写，因此它甚至可以写成

        MyControl.TabPages.Clear();

从那以后，我就代码审查问题受到质疑，发现我对为什么这是不好的做法的表述并不像我希望的那样有力。我说更难理解循环的流程，因此更难维护和调试，最终在代码的生命周期内成本更高。

是否有更好的说明为什么这是不好的做法？

Answer 1

唯一需要使用索引的原因就是选择性地擦除内容。即使在这种情况下，我认为最好说：

 i=0;
  而（我 而不是在每次删除后对循环索引进行金星。或者，更好的是，让索引向下计数，以便在删除项目时不会影响未来需要删除的项目的索引。如果删除了许多项目，这也可能有助于最大限度地减少每次删除后移动项目所花费的时间。
    

Answer 2

我同意你的挑战。如果他们想保留一个for循环，代码：

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

减少如下：

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

然后到：

for ( ; 0 < this.MyControl.TabPages.Count ; ) {
    this.MyControl.TabPages.Remove ( this.MyControl.TabPages[0] );
}

但如果存在 while 循环或 Clear() 方法，显然更可取。

Answer 3

我认为您可以通过引用 Knuth 的 literate programming 概念来构建一个更有力的论点，即程序不应该为计算机编写，而是要与其他程序员交流概念，因此更简单的循环：

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

更清楚地说明了意图 - 删除第一个标签页，直到没有留下任何标签页。我想大多数人会比原来的例子快得多。

Answer 4

我认为指出循环迭代不是由“i++”控制的事实，正如任何人所期望的那样，而是由疯狂的“i--”设置控制就足够了。

我还认为通过计算计数来改变“i”的状态，然后在循环中改变计数也可能导致潜在的问题。我希望 for 循环通常具有“固定”的迭代次数，并且 for 循环条件的唯一部分更改为循环变量“i”。

Answer 5

原始代码高度冗余，无法将 for 循环的操作转变为必要的。增量是不必要的，并由减量平衡。这些应该是前增量，而不是后增量，因为从概念上讲后增量是错误的。与标签页数的比较是半冗余的，因为这是一种检查容器是否为空的骇人听闻的方式。

简而言之，这是不必要的聪明，它增加而不是消除冗余。既然它可以既明显更简单又明显更短，那就错了。

Answer 6

一切都与期待有关。

当使用循环计数器时，您希望循环的每次迭代都会以相同的数量递增（递减）。

如果你弄乱了循环计数器（或者如果你喜欢猴子），你的循环不会像预期的那样运行。这意味着它更难理解，它增加了你的代码被误解的机会，这会引入错误。 或者（错误）引用一个聪明但虚构的人物：

complexity leads to misunderstanding

misunderstanding leads to bugs

bugs leads to the dark side.

Answer 7

可以使用的一个论点是调试这样的代码要困难得多，因为索引被更改了两次。

Answer 8

这可能更清楚：

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

Answer 9

我觉得你的发音很好。或许可以这样写：

既然逻辑可以表达很多 更清楚，它应该。

Answer 10

好吧，这增加了一些混乱，你可以很容易地写：

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

或（更简单）

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

或（最简单的）

MyControl.TabPages.Clear();

在上述所有情况下，我不必眯着眼睛去思考任何极端情况；很清楚什么时候会发生什么。如果你正在修改循环索引，你可以很快让它看起来很难理解。