17

Steve McConnell 的清单项目之一是您不应该胡乱使用循环索引（第 16 章，第 25 页，循环索引，PDF 格式）。

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

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

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

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

这个循环可以写成

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

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

        MyControl.TabPages.Clear();

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

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

4

10 回答 10

24

我觉得你的表达很棒。或许可以这样写：

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

于 2009-01-19T09:47:30.870 回答
23

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

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

或（更简单）

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

或（最简单的）

MyControl.TabPages.Clear();

综上所述，我不必眯着眼睛去思考任何极端情况。很清楚什么时候会发生什么。如果您正在修改循环索引，您很快就会使其一目了然。

于 2009-01-19T09:47:15.143 回答
16

这一切都与期待有关。

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

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

complexity leads to misunderstanding

misunderstanding leads to bugs

bugs leads to the dark side.
于 2009-01-19T09:57:41.753 回答
3

我同意你的挑战。如果他们想保留一个 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()方法，显然更可取。

于 2009-01-19T10:12:37.790 回答
2

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

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

更清楚地说明了意图 - 删除第一个标签页，直到没有留下任何标签页。我认为大多数人会比原始示例更快地理解。

于 2009-01-19T09:50:08.493 回答
1

这可能更清楚：

while (this.MyControl.TabPages.Count > 0)
{
  this.MyControl.TabPages.Remove ( this.MyControl.TabPages[0] );
}
于 2009-01-19T09:51:52.140 回答
1

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

于 2009-01-19T09:52:13.247 回答
1

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

简而言之，这是不必要的聪明，它增加而不是消除冗余。由于它既可以明显更简单，也可以明显更短，这是错误的。

于 2009-01-19T10:03:28.153 回答
1

使用索引的唯一原因是如果有人选择性地擦除事物。即使在那种情况下，我认为最好说：

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

于 2010-07-12T22:11:59.390 回答
0

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

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

于 2009-01-19T10:06:25.590 回答