重构两个几乎相同的方法

Question

我有两个几乎相同的方法来过滤列表并返回过滤后的结果。他们的算法是相同的，不同的是第一个函数返回最频繁的元素，第二个返回最不频繁的元素：

private static List<List<Character>> filter(List<List<Character>> lines, int charIndex) {
        List<List<Character>> result = copyList(lines);

        List<List<Character>> startWith0 = new ArrayList<>();
        List<List<Character>> startWith1 = new ArrayList<>();

        for(int i = 0; i < result.size(); i++) {
            List<Character> currentLine = result.get(i);
            if (currentLine.get(charIndex) == '1') {
                startWith1.add(currentLine);
            } else if (currentLine.get(charIndex) == '0') {
                startWith0.add(currentLine);
            }
        }

        if (startWith1.size() > startWith0.size() ||
        startWith1.size() == startWith0.size()) {
            return startWith1;
        } else {
            return startWith0;
        }
    }

第二个函数的结尾是这样的：

if (startWith1.size() > startWith0.size() ||
        startWith1.size() == startWith0.size()) {
        return startWith0;
} else {
        return startWith1;
}

我认为这种代码重复不是一个好的程序设计，但我没有看到将函数的第一部分和第二部分分成不同方法的好方法。

Answer 1

你有几个选择。

1. 辅助方法

创建一个辅助方法（它是private，它的名称以它是辅助方法的方法开头，尽管在这种情况下，它作为 2 的辅助方法，这可能有点棘手），它完成了所有工作，并有一个状态参数，指示如何做最后一部分。在这种情况下，您的状态参数可以简单地是 boolean。对于其他此类情况，通常使用枚举（您也可以将其写入同一个源文件，并声明为private）更合适。

这个助手会以这样的方式结束：

boolean winner = startWith1.size() < startWith0.size();
return most == winner ? startWith1 : startWith0;

你当前的filter 方法变成了单行：

public List<Character> filterMost(List<List<Character>> lines, int charIdx) {
  return filter(lines, charIdx, true);
}

private List<Character> filter(List<List<Character>> lines, int charIdx, boolean most) {
 ...
}

1. Lambdas

您可以传递一个函数，该函数根据 2 个列表的输入来选择要执行的操作。实际上它与第一个答案相同（仍然涉及辅助方法），但不是在辅助方法中编写不同的代码路径，而是在实际方法中编写它们（将它们传递给辅助方法）。它更复杂，但可以更容易地维护代码。通常，“更短、更简单”的代码总是比“更长、更复杂但理论上更易于维护”的代码更胜一筹，所以在这种情况下，我怀疑这是正确的做法。但是，在有更多状态并且涉及更多不同部分的情况下，这可能是更好的答案。看起来像这样：

public List<Character> filterMost(List<List<Character>> lines, int charIdx) {
 ...
}

public List<Character> filterLeast(List<List<Character>> lines, int charIdx) {
  return filter(lines, charIdx, (list0, list1) -> {
    if (list0.size() < list1.size()) return list0;
    return list1;
  };
}

private List<Character> filter(List<List<Character>> lines, int charIdx, BinaryOperator<List<Character>> op) {
 ...

return op.apply(startWith0, startWith1);
}

Answer 2

恕我直言，您可以排除填充两个 ArrayList 的循环，并且 - 当然 - 给这两种方法提供自我解释的名称，例如 filerMostFrequent/filterLeastFrequent 左右：

    private static List<List<Character>> filerMostFrequent(List<List<Character>> lines, int charIndex) {
        List<List<Character>> result = copyList(lines);

        List<List<Character>> startWith0 = new ArrayList<>();
        List<List<Character>> startWith1 = new ArrayList<>();

        filter(charIndex, result, startWith0, startWith1);

        if (startWith1.size() > startWith0.size() ||
        startWith1.size() == startWith0.size()) {
            return startWith1;
        } else {
            return startWith0;
        }
    }

    private static List<List<Character>> filterLeastFrequent (List<List<Character>> lines, int charIndex) {
        List<List<Character>> result = copyList(lines);

        List<List<Character>> startWith0 = new ArrayList<>();
        List<List<Character>> startWith1 = new ArrayList<>();

        filter(charIndex, result, startWith0, startWith1);

        if (startWith1.size() > startWith0.size() ||
                startWith1.size() == startWith0.size()) {
                return startWith0;
        } else {
                return startWith1;
        }
    }

    private static void filter(int charIndex, List<List<Character>> result, 
            List<List<Character>> startWith0,
            List<List<Character>> startWith1) {
        
        for(int i = 0; i < result.size(); i++) {
            List<Character> currentLine = result.get(i);
            if (currentLine.get(charIndex) == '1') {
                startWith1.add(currentLine);
            } else if (currentLine.get(charIndex) == '0') {
                startWith0.add(currentLine);
            }
        }
    }