《代码之丑》学习总结（上）

《代码之丑（极客时间）》是郑晔老师在极客时间上的一门课，专门讲述日常开发中比较典型的代码坏味道。这门课我很早就学完了，但一直没有来得及总结。其他课也一样，这是我个人的问题。最近我又学了一遍，借此机会将其主要内容进行梳理和归纳，锁定学习闭环。

郑晔老师讲了 13 种常见的编码陋习，其中很多内容是对《重构（豆瓣）》这本书的提炼，但结合现实案例后，对我们日常编码具有非常大的冲击力。我们可能一直在错误道路上行走，把错误的编码方式当成了习惯，但只要理解一点基本的底层逻辑，也许就能颠覆认知，再次成长。

1 没有业务含义的命名

（1）宽泛不精准的命名

这类命名，常见的有：data，info，flag，process，handle，build，maintain，manage，modify 等，当我们看到这些词时，脑子要警觉起来。太宽泛会带来什么问题呢？就是理解这段代码的成本高，你需要把代码读完才能知道它干了啥。

虽然「望文生义」是一个贬义词，但在软件命名上，我们就要「望文生义」。一看到函数名，就能理解，而不用消耗脑细胞去读细节。

public void processChapter(long chapterId) {
  Chapter chapter = this.repository.findByChapterId(chapterId);
  if (chapter == null) {
    throw new IllegalArgumentException("Unknown chapter [" + chapterId + "]");  
  }
  
  chapter.setTranslationState(TranslationState.TRANSLATING);
  this.repository.save(chapter);
}

上面示例程序的函数名称叫 processChapter，即处理章节，那处理了啥？我们不知道，因为名称太宽泛了。为此，我们必须去读代码，了解其逻辑是：根据章节 id 查询章节内容，然后将翻译状态改成“翻译中”，再保存。

那函数名称叫啥合适？代码逻辑是“将章节修改为翻译中”，不就是 changeChapterToTranslating？可这真是个好名称吗？还不是，因为它暴露太多细节了，我们封装类，封装函数的目的是什么？就是把细节隐藏起来。

一个好的名字应该描述意图（做什么），而非细节（怎么做）。

这也不行，那也不行，那你说啥名称合适？因为这里开启了一个翻译过程，所以其意图就是“开始翻译”，将其调整为 startTranslation 就是合适的。

（2）用技术术语命名

这类问题也很常见，比如 xxxList，xxxSet，xxxMap：

List<Book> bookList = bookService.findBooks();

那么问题出在哪呢？这是一种基于实现细节的命名方式，而不是前面说的意图。面向接口编程告诉我们，要用 List<Book> 而不是 ArrayList<Book>，那命名不也一样吗？如果有一天需要过滤重复，把 List<Book> 改成 Set<Book>，名称还叫 bookList 就不合适了。所以，更好的方案是：

List<Book> books = bookService.findBooks();

用技术方式命名，有时候还意味着缺乏一个模型，怎么理解？

Book cachedBook = redisBookStore.get(isbn);

这里的 redisBookStore 表示书籍信息在 redis 缓存中，但 redis 只是一种实现，这里应该有一个 cache 的缓存模型，即：

Book cachedBook = cache.get(isbn);

但 cache 本质上也是一个技术术语，还有更好的解决方案吗？Spring 中的 @Cacheable 注解是一个值得参考的案例：

@Cacheable("books")
public Book getByIsbn(String isbn) {
  ...
}

（3）用业务语言写代码

如果确实涉及到一些技术实现，比如通用的工具类，那使用技术术语没啥问题。但在业务系统中，大多数情况下都是业务，此时用技术语言就不合适了。如果想不到好名字，那只能说明还没理解业务：

public void approveChapter(long chaperId, long userId);

上面审核章节文章的方法有一个参数是 userId，为啥要一个 userId，写这段代码的人可能知道，但换个人，可能就是一脸懵，然后硬着头皮看代码，才发现原来是审核人的角色，记录是谁审核的。审核人用 userId 合适吗？为啥不用具备业务含义的 reviewerUserId ？

public void approveChapter(long chaperId, long reviewerUserId);

那怎么知道自己用的词到底是不是业务语言呢？如果不确定，就把这个词发给身边的产品经理，看 Ta 们能否看懂。另外，团队内要指定一套业务语言表，英文的，让大家保持一致。

2 乱用英文的命名

用拼音进行命名的代码已经突破底线了，我们这里不聊。

（1）语法错误

一般来说，类名是一个名词，表示一个对象；

而方法名则是一个动词，或者是动宾短语，表示一个动作。

名词可能相对简单一些，如果类名使用了动词说明用错了，需要调整；

方法名则是动词或动宾短语，其中动宾短语可能更容易出错：

// 重新翻译文章章节，使用名词 ❌，应该调整为 retranslate
public void retranslation(long chapterId);

// 完成翻译，使用完成时 ❌，应该调整为 completeTranslation
public void completedTranslate(long chapterId);

（2）不准确的词汇

作品审核使用 audit 还是 review？

对于不确定的词，查字典是最好的方式，但如果字典有多个不同的词怎么办？

集体智慧往往高于个人水平，听听大家的意见，然后把这些词添加到团队词汇表中。

（3）拼写错误

现在大多数 IDE 都有单词拼写检查，所以只要稍微注意一下，就能避免。另外，如果可以，代码中的注释、readme 等文档可以强制自己使用英文写，时间长了，语感会有所提升。

如何避免这些问题？制定代码规范、制定团队词汇表以及经常进行代码评审。

3 重复代码

重复代码怎么来的？大多数时候，可能源于我们天天敲击的 Ctrl-C 和 Ctrl-V。如何破？

不使用复制粘贴，如果一个函数需要被反复调用，就要将其提取出来，放到一个模型中。

也许有人说，复制粘贴没问题啊，很好用。但如果修改呢？软件设计的本质就是应对变化。如果只改你复制的部分还好，若要修改所有类似的代码呢？一旦一个地方漏改，直接凉凉～

复制粘贴的代码，结合 IDE 提示，很好识别。而代码结构重复，可能就没那么容易了：

@Task
public void sendBook(){
  try{
    bookService.sendBook();
  }catch(Exception ex){
    notifyService.sendMsg(new sendError(ex));
    throw ex;
  }
}

@Task
public void startTranslation(){
  try{
    bookService.startTranslation();
  }catch(Exception ex){
    notifyService.sendMsg(new sendError(ex));
    throw ex;
  }
}

上面两个函数，咋一看还比较简洁，但它们在结构上是重复的，主要体现在异常处理那部分。而优化的方式就是将其抽象成一个通用的结构：

@Task
public void sendBook(){
  executeTask(bookService::sendBook);
}

@Task
public void startTranslation(){
  executeTask(bookService::startTranslation);
}

private void executeTask(final Runnable runnable){
  try{
    runnable.run();
  }catch(Exception ex){
    notifyService.sendMsg(new sendError(ex));
    throw ex;
  }
}

还有一种重复，在 if...else if...else 结构化逻辑中经常出现，比如：

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

这种重复，让人不好理解的原因就在于没有看出开发人员想表达啥，也就是意图不明，换一个同事来维护，也得去读细节代码。另外，如果 editChapter 函数的参数再多两个，想发现二者之间的差异可能就得一个一个参数的对比了。

那如何表现出意图？很简单，把判断条件单独移出来：

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

这样一改，我们立马就能明白，当用户是一个编辑的角色时，可以对章节进行审核通过。如果将来条件变了，逻辑越来越复杂，那也没有关系，我们只要聚焦到 approved 这个变量的获取上即可，比如将其封装到一个独立的函数中：

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


private boolean isApproved(final User user){
  // 这里可以添加更复杂的逻辑判断
  return user.isEditor();
}

说到重复，就绕不开 DRY（Don’t Repeat Yourself） 原则：

在一个系统中，每一处知识都必须有单一、明确、权威地表述。

Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.

复制粘贴虽然一时爽，可后续变更就如同陷入泥沼，眼睁睁地看着时间一点一点被消耗殆尽。

有时候，慢，就是快。

4 长函数

曾经见过 3000 行的函数，这些都已经突破底线，我们不讨论。

那多长的函数才算长？每个人，每个团队可能都有不同的答案，但这个值不能太大，比如 100，所以是越小越好。郑晔老师对自己的要求是：

像Python，Ruby 等动态语言，由于其表达能力强，一行代码可以解决很多问题，所以一般情况下，函数的长度在 5 行左右；对于像 Java 这样表达能力弱一点的静态语言，一般在 10 行左右。

在很多业务逻辑中，可能 10 行搞不定，那么降低一点标准，翻一番，即 20 行。也就是说，大多数情况下，我们应该在 10 行内解决问题，特殊情况可以容忍到 20 行。

这可以作为一把尺子，衡量我们自己所写代码的粗细程度：对于长函数容忍度高，会导致长函数一直滋生。而究其原因，主要有以下三种：

• 以性能为由，不愿意把函数拆小

  拆分方法就涉及到函数的出入栈操作，很多人以此为由，拒绝拆分。但这个理由在当下大多数场景下是站不住的。一方面，硬件机器性能以摩尔定律再往前翻番；另一方面，编译器，解释器等都在进化，以 JVM 为例，内部自带优化，会对很多方法进行内联操作。

  更重要的是，写代码第一考量要素是可读性，可维护性，而不是性能。如果追求性能，换个人接手你写的代码，可能完全看不懂，对团队来说，经济成本更高。

• 平铺直叙写代码，多个业务流程，不同层面的细节都搅在一起

  public void executeTask() {
      ObjectMapper mapper = new ObjectMapper();
      CloseableHttpClient client = HttpClients.createDefault();
      List<Chapter> chapters = this.chapterService.getUntranslatedChapters();
      for (Chapter chapter : chapters) {
          // Send Chapter
          SendChapterRequest sendChapterRequest = new SendChapterRequest();
          sendChapterRequest.setTitle(chapter.getTitle());
          sendChapterRequest.setContent(chapter.getContent());
  
          HttpPost sendChapterPost = new HttpPost(sendChapterUrl);
          CloseableHttpResponse sendChapterHttpResponse = null;
          String chapterId = null;
          try {
              String sendChapterRequestText = mapper.writeValueAsString(sendChapterRequest);
              sendChapterPost.setEntity(new StringEntity(sendChapterRequestText));
              sendChapterHttpResponse = client.execute(sendChapterPost);
              HttpEntity sendChapterEntity = sendChapterPost.getEntity();
              SendChapterResponse sendChapterResponse = mapper.readValue(sendChapterEntity.getContent(), SendChapterResponse.class);
              chapterId = sendChapterResponse.getChapterId();
          } catch (IOException e) {
              throw new RuntimeException(e);
          } finally {
              try {
                  if (sendChapterHttpResponse != null) {
                      sendChapterHttpResponse.close();
                  }
              } catch (IOException e) {
                  // ignore
              }
          }
  
          // Translate Chapter
          HttpPost translateChapterPost = new HttpPost(translateChapterUrl);
          CloseableHttpResponse translateChapterHttpResponse = null;
          try {
              TranslateChapterRequest translateChapterRequest = new TranslateChapterRequest();
              translateChapterRequest.setChapterId(chapterId);
              String translateChapterRequestText = mapper.writeValueAsString(translateChapterRequest);
              translateChapterPost.setEntity(new StringEntity(translateChapterRequestText));
              translateChapterHttpResponse = client.execute(translateChapterPost);
              HttpEntity translateChapterEntity = translateChapterHttpResponse.getEntity();
              TranslateChapterResponse translateChapterResponse = mapper.readValue(translateChapterEntity.getContent(), TranslateChapterResponse.class);
              if (!translateChapterResponse.isSuccess()) {
                  logger.warn("Fail to start translate: {}", chapterId);
              }
          } catch (IOException e) {
              throw new RuntimeException(e);
          } finally {
              if (translateChapterHttpResponse != null) {
                  try {
                      translateChapterHttpResponse.close();
                  } catch (IOException e) {
                      // ignore
                  }
              }
          }
      }
  

  看到上面这段代码，很多人就会头疼，因为从函数名完全看不出干了啥，意图不明。只能啃细节：找到没有翻译的章节发给翻译引擎，由于翻译引擎是以 HTTP 服务形式提供，所以又包含了 HTTP 请求相关内容。一部分是业务逻辑，一部分又是技术细节（比如 http request 初始化，对象序列化成 json 等），这就是前面说的逻辑纠缠，职责不清。

  解决这个问题也很简单，就是提取函数，把各自的逻辑拆出来，而不是像面条一样放在一起。另外，因为函数长，想要体现出业务逻辑，函数名、变量名自然也跟着变长。但拆小以后，这些问题就自然消失了。此刻，你也许发现了，复杂度怎么来的？往往是我们自己种下的恶。

• 一次加一点点，最终把团队压死

  在前人留下的逻辑中每次加一点点，最终也可能会导致大厦将倾，比如：

  if (code == 400 || code == 401){
    ...
  }
  

  几次之后：

  if (code == 400 || code == 401 || code == 403 || code == 404
     || code == 503 || ...){
    ...
  }
  

  谁在作恶？可能大家都是无辜的，但每次哪怕只加一个条件，时间一长，代码就开始快速腐烂。面对这类问题，Bob 大叔引入了美国童子军一条简单的军规：

  Leave the campground cleaner than you found it.

  让营地比你来时更干净。

  如果你的改动让代码腐烂地更快，那么请立即改进它。
  
  

5 大类

前面讲了长函数，现在聊聊大类。本质上它们是一类问题，就是抬高了人对复杂问题的理解。人类的认知能力是有限的，大脑 cover 不住复杂问题。怎么办？分而治之，也就是任务分解，拆大化小。

拆模块，拆包，拆类，拆函数，所有这些都是为了分而治之，降低理解难度，提高代码的可读性和可维护性。当你看到模块名，包名，类名，函数名就能知道它们是干什么的（意图），而无需投入细节。

大类的坏味道一目了然，解决方案也简单：拆小。可问题是，怎么拆？面对这个问题，我们可以反过来想，如果知道了大类是怎么产生的，再逆向操作，不就是拆小？

• 职责不单一

  一个类里面的字段很多，但它们表达的业务有较大差异，比如下面这段代码：

  public class User {
    private long userId;
    private String name;
    private String nickname;
    private String email;
    private String phoneNumber;
    private AuthorType authorType;
    private ReviewStatus authorReviewStatus;
    private EditorType editorType;
    ...
  }
  

  前面几个字段（userId, name…）表示用户信息，中间部分（authorType, authorReviewStatus）表示用户作为作者角色的信息，而最后几个字段（editorType…）则表示用户作为编辑角色的信息。那么这个类到底想表达啥？用户，作者，还是文章编辑？

  职责不单一，不明确，从而导致类不断膨胀，而我们要做的，就是将它们拆开。

  public class User {
    private long userId;
    private String name;
    private String nickname;
    private String email;
    private String phoneNumber;
    ...
  }
  
  public class Author {
    private long userId;
    private AuthorType authorType;
    private ReviewStatus authorReviewStatus;
    ...
  }
  
  public class Editor {
    private long userId;
    private EditorType editorType;
    ...
  }
  

  拆开之后，每个小类就相对稳定了，而不像之前那样，User/Author/Editor 任何局部信息变化都会导致 User 大类改变。也就是说，一开始的 User 大类是易变的，Kent Beck 曾指出：

  每个类其实就是这样一个声明：这些逻辑应该放在一起，它们的变化不像它们所操作的数据那么频繁；这些数据也应该放在一起，它们变化的频率差不多，并且由与之关联的逻辑来负责处理。

  没错，变化频率，上面之所以把它们拆开，就是因为它们的变化频率不同。

• 字段未分组

  相对于前面依据职责不同的划分，这种坏味道可能没那么容易被挖出来，因为它要做的是对不同维度信息的封装。继续看上面拆分过的 User 小类：

  public class User {
    private long userId;
    private String name;
    private String nickname;
    private String email;
    private String phoneNumber;
    ...
  }
  

  有问题吗？没错，就是 email 和 phoneNumber，它们是什么？联系信息：

  public class User {
    private long userId;
    private String name;
    private String nickname;
    private Contact contact;
    ...
  }
  
  public class Contact {
    private String email;
    private String phoneNumber;
    ...
  }
  

  类的三大特性，大家都如数家珍，可真正理解并能在工作中实践落地，却不容易，比如这里的封装。本质上，拆大变小，是一个软件设计的工作，这里涉及到的就是单一职责原则。

如果想追求极致，可以看看 Jeff Bay 编写的对象健身操，一共 9 条规则：ThoughtWorks文集-第1章。

6 长参数列表

笔者之前单独总结过，请看这里。

7 滥用控制语句

滥用 if...else... 这样的控制语句，你会想到什么样的场景？

（1）嵌套

课程中提供了一个稍微简洁一点的案例：

public void distributeEpubs(final long bookId) {
  List<Epub> epubs = this.getEpubsByBookId(bookId);
  for (Epub epub : epubs) {
    if (epub.isValid()) {
      boolean registered = this.registerIsbn(epub);
      if (registered) {
        this.sendEpub(epub);
      }
    }                                            
  }
}

如何消除这些缩进，需要我们理解整个逻辑。比如 for 循环中一个个元素的处理逻辑，就是分发 epub 格式的电子书。我们把它单独拆出来：

public void distributeEpubs(final long bookId) {
  List<Epub> epubs = this.getEpubsByBookId(bookId);
  for (Epub epub : epubs) {
     this.distributeEpub(epub);                                          
  }
}

public void distributeEpub(final Epub epub) {
  if (epub.isValid()) {
      boolean registered = this.registerIsbn(epub);
      if (registered) {
        this.sendEpub(epub);
      }                                       
  }
}

至多一层缩进，拆出来后，再看原来的 distributeEpubs 函数，简单清晰。

（2）if else

前面提到的对象健身操中有一条：

不要使用 else 关键词，应该使用卫语句替代嵌套的条件表达式，即当条件不满足时，立即从函数中返回。

为此，我们继续对上面的 distributeEpub 重构：

public void distributeEpub(final Epub epub) {
  if (!epub.isValid()) {
      return;
  }
      
  boolean registered = this.registerIsbn(epub);
  if (!registered) {
      return;
  }

  this.sendEpub(epub);
}

当消除了多层缩进后，理解这段代码的难度瞬间就小了，但如果有很多层 if...else if...else 呢？

public double getEpubPrice(final boolean highQuality, final int chapterSequence) {
  double price = 0;
  if (highQuality && chapterSequence > START_CHARGING_SEQUENCE) {
    price = 4.99;
  } else if (sequenceNumber > START_CHARGING_SEQUENCE
        && sequenceNumber <= FURTHER_CHARGING_SEQUENCE) {
    price = 1.99;
  } else if (sequenceNumber > FURTHER_CHARGING_SEQUENCE) {
    price = 2.99;
  } else {
    price = 0.99;
  }
  
  return price;
}

还按照方式改吗？可以，但却不是最优解，因为这种逻辑本质上是缺少一个模型，应该拆出一个接口，然后通过多态解决。

（3）循环

循环有什么问题？没啥大问题，就是有些过时了，因为函数式编程的普及，我们应该用更现代的方式来写循环语句。Java 8 中引入 Stream 流就是干这个事的：

public void distributeEpubs(final long bookId) {
  List<Epub> epubs = this.getEpubsByBookId(bookId);
  epubs.stream()
    .filter(Epub::isValid)
    .filter(this::registerIsbn)
    .forEach(this::sendEpub);
}

在《重构 (豆瓣)》第一版中并未提及循环语句有问题，但在最新的第二版中却将其纳入其中，为什么？上面的写法到底有什么优势？代码的抽象层次不同。

循环语句表现的是细节（怎么做），而流中的列表转换表现的是意图（做什么）。

（4）switch

跟上面 if...else if...else 雷同的，就是重复的 switch 语句，比如：

public double getBookPrice(final User user, final Book book) {
  double price = book.getPrice();
  switch (user.getLevel()) {
    case UserLevel.SILVER:
      return price * 0.9;
    case UserLevel.GOLD: 
      return price * 0.8;
    case UserLevel.PLATINUM:
      return price * 0.75;
    default:
      return price;
  }
}

前面说过，这里少了一个模型：

interface UserLevel {
  double getBookPrice(Book book);
}

class RegularUserLevel implements UserLevel {
  public double getBookPrice(final Book book) {
    return book.getPrice();
  }
}

class GoldUserLevel implements UserLevel {
  public double getBookPrice(final Book book) {
    return book.getPrice() * 0.8;
  }
}

...

如果想更简约一点，可以使用枚举类的特性来实现：

public enum UserLevel {
    SILVER() {
        @Override
        public double getPrice(Book book) {
            return book.getPrice() * 0.9;
        }
    },
  
    GOLD() {
        @Override
        public double getPrice(Book book) {
            return book.getPrice() * 0.8;
        }
    },
  
    PLATINUMP() {
        @Override
        public double getPrice(Book book) {
            return book.getPrice() * 0.75;
        }
    };

    public abstract double getPrice(Book book);
}

小结

这篇小结中，主要对日常开发中的命名，重复，长函数，长参数列表，大类，控制语句滥用等问题进行了剖析，如果你已经发现了相关问题，那么立即动手调整是一个好习惯。

大多数时候，我们的主要精力是在看代码和理解代码上，所以写代码时，可读性和可维护性，是第一考量。这也是为什么前文中反复在强调意图，强调做什么，而不是怎么做。一个好的名字，可能会帮我们节约半个小时甚至几个小时。那意图又是什么呢？是对业务的深刻理解。

另一方面，人脑的认知有限，把复杂的内容拆解成一个个小部分，那么这些小部分所造成的影响也将是局部的。既然是局部影响，那便可控，造成重大软件事故的概率就小。