如何重构多个函数中的重复循环答案

【问题标题】：How to refactor duplicate loops in multiple functions如何重构多个函数中的重复循环
【发布时间】：2013-11-26 12:17:06
【问题描述】：

我正在尝试 TDD 教程并想编写好的代码。我遇到了使用循环重复代码的问题。

我的代码如下所示：

public Board(int rows, int columns) {
    this.rows = rows;
    this.columns = columns;

    blocks = new Block[rows][columns];

    for (int row = 0; row < rows; row++) {
          for (int col = 0; col < columns; col++) {
              blocks[row][col] = new Block('.');
          }
    }
}

public boolean hasFalling(){
    boolean falling = false;

    for (int row = 0; row < rows; row++) {
        for (int col = 0; col < columns; col++) {
            if(blocks[row][col].getChar() == 'X'){
                falling = true;
            }
        }
    }

  return falling;
}

 public String toString() {
    String s = "";
    for (int row = 0; row < rows; row++) {
        for (int col = 0; col < columns; col++) {
            s += blocks[row][col].getChar();
        }
        s += "\n";
    }
    return s;
}

如您所见，我在不同的方法中使用相同的 for 循环。有没有办法避免这种情况以及如何避免这种情况？

我正在使用 Java 编程。

【问题讨论】：

您应该摆脱boolean falling = false;，将falling = true; 更改为return true;，并将return falling; 更改为return false;...不是问题的答案，只是评论。 ..
@nhgrif 这只是个人喜好问题。我个人会像他一样这样做，因为我只在方法的末尾（有一些例外）返回语句
@mvieghofer 如果您只使用一个返回值，那么至少在 if 语句中放置一个中断，这样当您已经知道答案时就不会循环遍历矩阵的其余部分
主要的一点是他应该在他知道他可以return true 的那一刻完全跳出for 循环，并避免检查每个元素。
@dkatzel 是的。但是他还需要在inner for loop 之后使用if 语句来执行if(falling){break;} 以避免迭代更多的外部循环。简单地来自inner loop 的return true-ing 更干净，imo。

标签： java loops refactoring duplicates

【解决方案1】：

我认为您对优秀代码的“避免代码重复”的想法有点过于严肃了。确实，您应该避免重复代码，因为它会使您的代码更难阅读和维护。但是循环是控制语句，不需要避免。它类似于if 语句，尽管您会在代码中多次使用这些语句，但您不会将 if 放入额外的方法中。

尽管如此，如果你真的想这样做，你可以为 for 循环中的每个代码块创建一个 Runnable 并创建一个这样的方法：

public void loop(Runnable runnable) {
    for (int row = 0; row < rows; row++) {
      for (int col = 0; col < columns; col++) {
          runnable.run();
      }
    }
}

然后您可以将所需的 Runnable 传递给该方法（您可能还需要以某种方式将参数传递给 runnable）。有关更多信息，请参见例如this post on SO.

【讨论】：

这种看起来像访客模式的简化版本（但没有树）。您实际上是在遍历期间添加了一个事件处理程序。我喜欢！
是的，基本上就是访问者模式。

【解决方案2】：

我不确定如何简化所有循环（我也不完全确定您需要/想要），但您基本上可以消除 hasFalling() 方法中的大部分代码。相反，您可以这样做：

public boolean hasFalling(){
   return toString().contains('X');
}

【讨论】：

它真的有效吗？哦，我的上帝.. 匆忙打开 Eclipse！

【解决方案3】：

注意两点：

如果您在多个位置有完全相同的 for loop 块，则应将其分解为函数。

另外，如果你有完全相同的循环，你的架构会很糟糕。您只需运行循环一次，并使其可用于关心它的事物 - 可能是通过一个 getter 函数，该函数会将循环的结果返回给任何需要它的东西。

在你的情况下，所有这些都是无关紧要的。你的for loops 不一样。仅仅因为你有循环在这里并不重要......你对循环所做的事情是不同的。

您确实有一个大问题，使您的代码难以理解，并且会给您带来很多困难。了解依赖注入。您的函数显然需要blocks 才能工作，但这在进一步检查函数之前还不清楚。如果您的函数需要（依赖于）blocks，则应将其传递给它。

像hasFalling(blocks) 这样重构，让事情更清楚。全局状态是一种糟糕的做法，从长远来看会毁了你。抛弃全局变量并声明你的依赖项。

【讨论】：