重构两个几乎相同的方法
Refactoring two almost identical methods
我有两个几乎相同的方法来过滤列表和 returns 过滤结果。他们的算法是一样的,不同的是第一个函数returns最频繁的元素,第二个returns最不频繁的元素:
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;
}
我认为这种代码重复不是一个好的程序设计,但我没有看到将函数的第一部分和第二部分划分为不同方法的好方法。
您有几个选择。
- 辅助方法
制作一个辅助方法(它是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) {
...
}
- Lambdas
您可以传递一个函数,该函数根据 2 个列表的输入选择要执行的操作。实际上它与第一个答案相同(仍然涉及辅助方法),但不是在辅助程序中编写不同的代码路径,而是在实际方法中编写它们(将它们传递给辅助程序)。它更复杂,但可以使代码更容易维护。一般来说,'shorter, less complex' 代码总是胜过 'longer, more complex, but theoretically more maintainable' 代码,所以在这种情况下,我怀疑这是正确的举动。但是,在状态更多且不同部分涉及更多的情况下,这可能是更好的答案。看起来像这样:
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);
}
恕我直言,您可以分解出填充两个 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);
}
}
}
我有两个几乎相同的方法来过滤列表和 returns 过滤结果。他们的算法是一样的,不同的是第一个函数returns最频繁的元素,第二个returns最不频繁的元素:
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;
}
我认为这种代码重复不是一个好的程序设计,但我没有看到将函数的第一部分和第二部分划分为不同方法的好方法。
您有几个选择。
- 辅助方法
制作一个辅助方法(它是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) {
...
}
- Lambdas
您可以传递一个函数,该函数根据 2 个列表的输入选择要执行的操作。实际上它与第一个答案相同(仍然涉及辅助方法),但不是在辅助程序中编写不同的代码路径,而是在实际方法中编写它们(将它们传递给辅助程序)。它更复杂,但可以使代码更容易维护。一般来说,'shorter, less complex' 代码总是胜过 'longer, more complex, but theoretically more maintainable' 代码,所以在这种情况下,我怀疑这是正确的举动。但是,在状态更多且不同部分涉及更多的情况下,这可能是更好的答案。看起来像这样:
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);
}
恕我直言,您可以分解出填充两个 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);
}
}
}