《代码之丑》学习笔记03—— 重复代码：简单需求到处修改，怎么办？

本文链接：https://blog.csdn.net/qq2632246528/article/details/124366934

03 | 重复代码：简单需求到处修改，怎么办？

1.不要直接复制粘贴

复制粘贴是最容易产生重复代码的地方，所以，一个最直白的建议就是，不要使用复制粘贴。真正应该做的是，先提取出函数，然后，在需要的地方调用这个函数。

2.重复的结构

先看以下代码

@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;
  }
}

接下来提取相同结构后就可以这样写

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

3.做真正的选择

首先上例子

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

相信你和我一样，第一眼看到这段代码的感觉一定是，if 选择的一定是两段不同的业务处理。但只要你稍微看一下，就会发现，if 和 else
两段代码几乎是一模一样的。在经过仔细地“找茬”之后，才能发现，原来是最后一个参数不一样。

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

因为作者在写这段代码时，脑子只想到 if 语句判断之后要做什么，而没有想到这个 if 语句判断的到底是什么。但这段代码客观上也造就了重复。

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

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

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

只要你看到 if 语句出现，而且 if 和 else 的代码块长得又比较像，多半就是出现了这个坏味道。

同时这里还可以做一个优化

如果将来审核通过的条件改变了，变化的点全都在 approved
的这个变量的赋值上面。如果你追求更有表达性的做法，甚至可以提取一个函数出来，这样，就把变化都放到这个函数里了，就像下面这样：

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


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