一个简单需求竟让我改了十几处代码，万字控诉到底什么是重复代码！

Posted 2021-10-13 JavaEdge.

tags:

篇首语：本文由小常识网(cha138.com)小编为大家整理，主要介绍了一个简单需求竟让我改了十几处代码，万字控诉到底什么是重复代码！相关的知识，希望对你有一定的参考价值。

刚开始工作时，总有人开玩笑说，编程实际上就是 CV，调侃很多程序员写程序依靠的是复制粘贴。

至今，很多初级甚至高级程序员写代码依旧是CV，就是把其他项目里的一段代码复制过来，稍加改动，然后，跑一下没有大问题就完事。
这就是在给子孙后代挖坑！

通常只要这些复制代码其中有一点逻辑要修改，就意味着所有复制粘贴的地方都要修改。所以实际项目中，常常发现明明很简单的需求，你却要改很多地方，需要花很长时间，搞的项目经理和产品经理，对你的进度都很不满意。

而且，只要你少改一处，就意味着留下一处潜在bug，预发布时发现又有问题，继续回退重新发版。

CV是最容易产生重复代码的地方，所以不要CV！
正确姿势是先提取出方法，然后，在需要的地方调用这个方法。

CV 的重复代码相对容易发现，但有一些代码有类似结构，这也是重复代码，有些人却对这类坏味道视而不见。
还有很多制造重复代码的不竭动力：

代码结构不合理导致同一个实现散落各处
由于初期代码结构设计不合理导致后续功能实现无法快速找到已有实现，或者找到了但是不好引用已有实现。
改进：初期设计代码逻辑合理，对于不合理的地方要及时重构，防止演变成下一个原因
为了稳定性，不动老逻辑
拷贝一份。由于对于业务的不熟悉和对自己代码能力的不信任不敢重构导致。
改进：通过微重构进行多次迭代小改进慢慢优化
互联网敏捷开发的“快”
由于时间紧张或者能力问题无法识别出的坏代码。
改进：提升能力

重复结构

@Task
public void sendBook() {
  try {
    this.service.sendBook();
  } catch (Throwable t) {
    this.notification.send(new SendFailure(t)));
    throw t;
  }
}

@Task
public void sendChapter() {
  try {
    this.service.sendChapter();
  } catch (Throwable t) {
    this.notification.send(new SendFailure(t)));
    throw t;
  }
}

@Task
public void startTranslation() {
  try {
    this.service.startTranslation();
  } catch (Throwable t) {
    this.notification.send(new SendFailure(t)));
    throw t;
  }
}

一个系统要把作品的相关信息发送给翻译引擎：

sendBook 把作品信息发出去
sendChapter 把章节发出去
startTranslation 启动翻译

这几个业务都是以后台执行，所以，方法签名都增加了一个 Task 的 Annotation，表是任务调度的入口。
然后，实际的代码执行放到了对应的service业务方法。

很多人觉得已经够简洁了，但这段代码结构却是有重复的，注意catch语句。

之所以要做一次捕获（catch），是为了防止系统出问题而无人发觉。捕获到异常后，把出错的信息通过即时通讯工具发给相关人等，代码里的 notification.send 就是发通知的入口。相比于原来的业务逻辑，这个逻辑是后来加上的，所以，这段代码的作者不厌其烦地在每一处修改了代码。

虽然这三个函数调用的业务代码不同，但它们的结构是一致的，其基本流程可以理解为：

调用业务函数
如果出错，发通知

当你能够发现结构上的重复，我们就可以把这个结构提取出来。从面向对象的设计来说，就是提出一个接口：

private void executeTask(final Runnable runnable) {
  try {
    runnable.run();
  } catch (Throwable t) {
    this.notification.send(new SendFailure(t)));
    throw t;
  }
}

对于支持函数式编程的程序设计语言，可以用语言提供的便利写法简化代码的编写，像下面的代码就是用了 Java 里的方法引用（Method Reference）：

@Task
public void sendBook() {
  executeTask(this.service::sendBook);
}
@Task
public void sendChapter() {
  executeTask(this.service::sendChapter);
}
@Task
public void startTranslation() {
  executeTask(this.service::startTranslation);
}

如果再有一些通用的结构调整，比如，在任务执行前后要加上一些日志信息，这样的改动就可以放到 executeTask 方法里，不用四处改！还容易漏了！

所以这个问题其实不复杂，关键在于发现结构重复。相比直接CV，结构重复极具迷惑性。很难让人一下反应出来干的三件事，居然也是重复代码。

一般参数是名词，而方法调用是动词。动词不同时，并不代表没有重复代码！懂得这点，就比较容易发现结构相似了。
比如案例中的：发作品信息、发章节、启动翻译，看上去是三件不同事，只是因为动词不同，但除了动词，其它部分都相同！所以，它们在结构上重复。

做真正的选择

if (user.isEditor()) {
  service.editChapter(chapterId, title, content, true);
} else {
  service.editChapter(chapterId, title, content, false);
}

对章节内容进行编辑。有一个业务逻辑，章节只有在审核通过之后，才能去做后续的处理，比如，章节的翻译。所以，这里的 editChapter 方法最后那个参数表示是否审核通过。

在这段代码里面，目前的处理逻辑:

如果这个章节是由作者来编辑的，那么这个章节是需要审核的
如果这个章节是由编辑来编辑的，那么审核就直接通过了，因为编辑本身同时也是审核人

问题来了，if 选择的到底是什么？

感觉if 选择的一定是两段不同业务处理。但只要你稍微看一下，就会发现，if 和 else 两段代码几乎一样，只是最后的一个参数不同。

只有参数不同，是不是和前面说的重复代码是如出一辙的？
没错，这也是一种重复代码。

只不过，这种重复代码通常情况下是作者自己写出来的，而不是粘贴出来的。
因为写这段代码时，脑子只想到 if 语句判断之后要做什么，而没有想到这个 if 语句判断的到底是什么。
这段代码客观上造就了重复。

写代码要有表达性。把意图准确地表达出来，是写代码过程中非常重要的一环。显然，这里的 if 判断区分的是参数，而非动作。所以，我们可以把这段代码稍微调整一下，会让代码看上去更容易理解：

boolean approved = user.isEditor();
service.editChapter(chapterId, title, content, approved);

这里我把 user.isEditor() 判断的结果赋值给了一个 approved 变量，而不是直接作为一个参数传给 editChapter，这么做也是为了提高这段代码的可读性。
因为 editChapter 最后一个参数表示的是这个章节是否审核通过。通过引入 approved 变量，我们可以清楚地看到，一个章节审核是否通过的判断条件是“用户是否是一个编辑”，这样代码更清晰！

若将来审核通过的条件改变了，变化点都在 approved 变量的赋值这一处而已！
如果你追求更有表达性的做法，甚至可以提取一个方法，这样，就把变化都放到这个方法里：

boolean approved = isApproved(user);
service.editChapter(chapterId, title, content, approved);

private boolean isApproved(final User user) {
  return user.isEditor();
}