重复代码

143 阅读5分钟

重复是一个泥潭,对于程序员来说,时刻提醒自己不要重复是至关重要的。

坏味道排查列表

  1. 复制粘贴的代码
  2. 结构重复的代码
  3. if和else代码块中的语句高度类似

解决方案

遵循DRY(Don't Repeat Yourself)原则
每一处知识都必须有单一、明确、权威的表述

举例说明

重复结构

有如下几段代码

@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 这个函数里,而不用四处去改写了。

if引出的重复代码

我们再来看一段代码:

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

这是一段对章节内容进行编辑的代码。这里有一个业务逻辑,章节只有在审核通过之后,才能去做后续的处理,比如,章节的翻译。所以,这里的 editChapter 方法最后那个参数表示是否审核通过。
在这段代码里面,目前的处理逻辑是,如果这个章节是由作者来编辑的,那么这个章节是需要审核的,如果这个章节是由编辑来编辑的,那么审核就直接通过了,因为编辑本身同时也是审核人。
按照正常的逻辑来说,if选择的一定是两段不同的业务逻辑。但是我们仔细看下,if和else两段代码几乎是一模一样的,只有最后一个参数不同,只有参数不同其实也算是一种重复结构的代码。
可能有人认为这就是两段不同业务逻辑的代码啊,一个是没有审批通过的逻辑,一个审批通过的逻辑。那这怎么分析呢?
首先我们要拉起对业务逻辑的观念,其实对于这个审批通过的编辑文章方法和审批不通过的文章编辑方法,本质上就是编辑文章的业务逻辑,所以算得上是一段重复的逻辑。
写代码要有表达性。把意图准确地表达出来,是写代码过程中非常重要的一环。显然,这里的 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();
}