重构-为什么 if-else 不是好代码
重构-为什么 if-else 不是好代码
平时开发中if-else用的多吗？ 其实这是个再正常不过的coding习惯，当我们代码量小的时候用来做条件判断是再简单不过的了。 但对于优秀程序员来说，这并不是好代码， 为啥？
抛开剂量谈毒性都是耍流氓
在使用条件判断语句的地方，如果代码量小，需要判断的场景少的话， 那么没有比 if-else 更合适的语句，比如下面这样
....
if(object.getIndex() > 0) {
//do something
} else {
//do other things
}那在什么情况下 if-else 才会变差呢？ 以上面的代码为例子，当需要判断的情况逐渐增加的时候，上面的代码可能会变的难以维护。 在进阶高级开发的路上，应该逐步培养起这种前瞻意识， 即使在代码还在起步阶段，应该要能够看到将来代码发展的趋势， 比如上面的代码，当情况越来越多的时候，if-else可能会发展出许多个分支
....
if(object.getIndex() > 200) {
//do something
} else if(object.getIndex() > 100) {
//do something
} else if(object.getIndex() > 0) {
//do something
} else if(object.getIndex() < 0) {
//do something
} else {
//do other things
}这是完全可能的，以我的经验来说就在不少项目上见过这样的代码。 而且代码执行块中的逻辑可能在几次迭代后变的非常复杂，就像下面这样
if (target.startsWith("#RANGE")) {
target = sheet.name() + target.replaceFirst("#RANGE", "");
hy.newDocumentLink();
} else if (target.contains("#RANGE")) {
// 并非完整的路径
String filePath = target.substring(0, target.indexOf('#'));
LinkedHashMap<KmoSheet, String> sheetSrcMap = tdHandler
.getTrHandler().getTableHandler().getBodyHandler()
.getReader().getSheetSrcMap();
Set<KmoSheet> sheets = sheetSrcMap.keySet();
for (KmoSheet sheet : sheets) {
if (sheetSrcMap.get(sheet).endsWith(filePath)) {
target = sheet.name()
+ target.substring(target.indexOf('!'),
target.length());
break;
}
}
hy.newDocumentLink();
} else if (target.startsWith("#")) {
hy.newDocumentLink();
} else if (target.contains("#")) {
// 并非完整的路径
String filePath = target.substring(0, target.indexOf('#'));
LinkedHashMap<KmoSheet, String> sheetSrcMap = tdHandler
.getTrHandler().getTableHandler().getBodyHandler()
.getReader().getSheetSrcMap();
Set<KmoSheet> sheets = sheetSrcMap.keySet();
for (KmoSheet sheet : sheets) {
if (sheetSrcMap.get(sheet).endsWith(filePath)) {
target = sheet.name()
+ '!'
+ target.substring(target.indexOf('#') + 1,
target.length());
break;
}
}
hy.newDocumentLink();
} else {
hy.newFileLink();
boolean isSingleFile = tdHandler.getTrHandler()
.getTableHandler().getBodyHandler().getReader()
.isSingleFile();
if (!isSingleFile) {
if (target.startsWith("../")) {
target = target.replaceFirst("../", "");
}
}
}看到这段代码第一感觉就是想杀个小伙伴祭天。
如何重构掉这段代码
对于这种代码我们重构的目标可以有两个深度，看自己强迫症的严重程度决定 · 继续用 if-else，只达到剥离执行代码块 · 用工厂模式去耦合 对于这两种其实不是非此即彼的关系，而是优化深度不同。第一种相对比较简单，可以重构成下面这样子
if (target.startsWith("#RANGE")) {
processStartWithTag();
} else if (target.contains("#RANGE")) {
processContainsWithTag();
} else if (target.startsWith("#")) {
processStartWithStash();
} else if (target.contains("#")) {
processContainsWithStash();
} else {
processOtherSituation();
}代码清爽了很多， 现在这段代码可以清楚的看出来都处理了哪些情况，条件判断的代码只关注了条件的不同， 而对于不同条件的具体处理逻辑我们剥离到了其他地方， 这样即使写到脑袋迷糊，也不至于说漏了哪个条件没判断。
进一步优化
在上面的优化之后，如何再用工厂模式来继续重构呢？ 从上的代码看的出来，不同的条件下，执行的逻辑是不同的，那么可以把这种执行逻辑抽象出来，用多态的概念来定义不同的执行方式。
public abstract class TargetExecutor {
public void process();
}
//处理target.startsWith("#RANGE")的情况
public class ExecutorWithTag extends TargetExecutor {
public void process() {
....
}
}
//处理target.contains("#RANGE")的情况
public class ExecutorContainsTag extends TargetExecutor {
public void process() {
....
}
}
....完成了这一步之后，就可以把代码块中不同条件下的方法抽到各个不同的具体类里面去了，
TargetExecutor executor;
if (target.startsWith("#RANGE")) {
executor = new ExecutorWithTag();
} else if (target.contains("#RANGE")) {
....
} else if (target.startsWith("#")) {
....
} else if (target.contains("#")) {
....
} else {
....
}
executor.process();还可以进一步优化吗？可以的，甚至这里的条件判断都可以不要，我们可以定义一个工厂来把 new ExecutorWithTag()这件事给包了，
public class ExecutorFactory {
public TargetExecutor getExecutor(String target) {
if (target.startsWith("#RANGE")) {
return new ExecutorWithTag();
} else if (target.contains("#RANGE")) {
....
} else if (target.startsWith("#")) {
....
} else if (target.contains("#")) {
....
} else {
....
}
}
}对工厂模式还有印象吗，上面这段代码在我之前的工厂模式一文里出现过，这里可以算是工厂模式的一个实际应用。 在经过这一轮重构之后，我们之前在一个类里面写的那堆代码已经抽离到多个不同的类里了， 现在在原来的类里的代码变成怎样了呢，
TargetExecutor executor = ExecutorFactory.getExecutor(target);
executor.process();重构之后各个Executor和主类中的耦合已经降到很低了， 而且代码整洁度提高了很多，之前那个类的一段50+行的代码变成了2行，这就是重构的意义。