坏味道

2021/01/12 2 分钟阅读

前言

不仅是代码有坏味道，在项目落地过程中，组织协同、沟通等都有坏味道。

通过一段时间的实践，笔者深切的感受就是：性能往往够用就行，可读性才是第一优先的。代码优化是一把双刃剑基于简洁的思想写成的源代码足以满足 99% 情况下的性能需求。而且这将极大地提高应用程序的可维护性。任何以性能为名使您的代码难以理解的编码实践都是不值得的。在没有可衡量的性能问题的情况下，您不应该进行优化，因为这是过早优化，就算你认为自己会获得性能上的提升也不要这样做。

让营地比你来时更干净。—— 童子军军规。我们应该看看自己对于代码的改动是不是让原有的代码变得更糟糕了，如果是，那就改进它。但这一切的前提是，你要能看出自己的代码是不是让原有的代码变得糟糕了。

只要能做到命名合理、没有重复、各个代码单元（类、函数等）体量适当、各个代码单元有明确且单一的职责、各个代码单元之间有恰当的交互，这就已经是质量相当高的代码了。对象健身操—-Thought Works文集提了9个规则

方法只使用一级缩进
拒绝 else 关键字
封装所有的原生类型和字符串
一行代码只有一个“ . ”运算符
不要使用缩写
保持实体对象简单清晰。每个类的长度都不能超过50 行，每个包所包含的文件不超过 10 个。
类中的实例变量都不要超过两个
使用一流的集合
不使用任何 Getter/Setter/Property 这套“健身操”的意义在于：“在一个简单的项目里尝试一些比以前严格得多的编码标准……会迫使你更为严格地以面向对象的风格编写代码”，从而“以一种全新的方式思考你的代码”。不过这得需要你刻意练习。正所谓“台上一分钟，台下十年功”，缺乏在受控环境下的刻意练习，很难通过工作中的自然积累提升判断力。

不好的命名

写程序和写文章有极强的相似性，本质都是用语言阐述一件事情。试想，如果文章用的都是一些词不达意的句子，这样的文章谁能看的懂，谁又愿意去看呢？

命名过于宽泛，不能精准描述，需要阅读这段代码的细节才懂。比如data、info、flag、process、handle、build、maintain、manage、modify 等等
用技术术语命名，比如 xxxMap、xxxSet。面向接口编程，不要面向实现编程，在命名上也是如此。在实际的代码中，技术名词的出现，往往就代表着它缺少了一个应有的模型。

好的命名要体现出这段代码在做的事情，能反馈它的实际作用，而无需展开代码了解其中的细节，这是最低的要求。再进一步，好的命名要准确地体现意图，而不是实现细节。更高的要求是，用业务语言写代码。

程序员必备的思维能力：抽象思维这也是为什么，我在做设计和代码审查（Code Review）的时候，会特别关注命名是否合理的原因。因为命名的好坏，在很大程度上反应了我们对一个概念的思考是否清晰，我们的抽象是否合理，反应在代码上就是，代码的可读性、可理解性是不是良好，以及我们的设计是不是到位。就像Stack Overflow的创始人Joel Spolsky所说的：“起一个好名字应该很难，因为，一个好名字需要把要义浓缩在一到两个词。（Creating good names is hard, but it should be hard, because a great name captures essential meaning in just one or two words）。”是的，这个浓缩的过程就是抽象的过程。我不止一次的发现，当我觉得一个地方的命名有些别扭的时候，往往就意味着要么这个地方我没有思考清楚，要么是我的抽象弄错了。

重复的代码

只要这些复制代码其中有一点逻辑要修改，就意味着所有复制粘贴的地方都要修改。更可怕的是，只要你少改了一处，就意味着留下一处潜在的问题。

重复的结构：一般来说，参数是名词，而函数调用，是动词。名词的重复是好判断的，但在函数式编程兴起之后，动词不同时，并不代表没有重复代码产生。

if 和 else 的代码块长得比较像

if (user.isEditor()) {
  service.editChapter(chapterId, title, content, true);
} else {
  service.editChapter(chapterId, title, content, false);
}
// ==> 
boolean approved = user.isEditor();
service.editChapter(chapterId, title, content, approved);

程序员必备的思维能力：抽象思维重复代码是典型的代码坏味道，其本质问题就是抽象的缺失。因为我们Ctrl+C加Ctrl+V的工作习惯，导致没有对共性代码进行抽取，或者虽然抽取了，只是简单的用了一个Util名字，没有给到一个合适的名字，没有正确的反应这段代码所体现的抽象概念，都属于抽象不到位。提取重复代码只是我们重构工作的第一步。对重复代码进行概念抽象，寻找有意义的命名才是我们工作的重点。

长函数和大类

平铺直叙的代码没有把不同的东西分解出来。如果我们用设计的眼光衡量，这就是“分离关注点”没有做好，存在的两个典型问题：

把多个业务处理流程放在一个函数里实现；
把不同层面的细节放到一个函数里实现。

很多代码难读，一个重要的原因就是把不同层面的代码混在了一起。

长函数往往有助于好的命名。因为变量都是在这个短小的上下文里，也就不会产生那么多的命名冲突，变量名就可以写短一些。

大类的产生

职责不单一
字段未分组

// 在这个类的设计里面，总有一些信息对一部分人是没有意义，但这些信息对于另一部分人来说又是必需的。
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;
  ...
}
// ==> 改为
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;
  ...
}
// ==> 改为
// 基本信息是那种一旦确定就不怎么会改变的内容，而联系方式则会根据实际情况调整
public class User {
  private long userId;
  private String name;
  private String nickname;
  private Contact contact;
  ...
} 
public class Contact {
  private String email;
  private String phoneNumber;
  ...
}

一个函数一般不要超过10行，类不要超过5个字段。分解大体积函数更重要的是便于阅读。其实，没有任何函数计算机也能运行得好好的，函数的存在只是为了服务于程序员，所以多多利用它们。对于分步骤执行的函数，将函数中的每个步骤都分解成子函数效果会更好。而对于其他如决策类的函数，不同的决策会引向不同的函数：有的部分负责制定决策，有的则是负责执行决策。分解函数的方法有很多种维度，只有通过不断的练习才能一眼看穿哪种才是正确的。

将大类拆解成小类，本质上在做的工作是一个设计工作。我们分解的依据其实是单一职责这个重要的设计原则。很多人写代码写不好，其实是缺乏软件设计的功底，不能有效地把各种模型识别出来。

长参数：宁可要十个零参数的小函数，也不要一个带十个参数的函数。

如果概念上一致的话，或者有相同的变化原因，可以将参数封装成一个类，并给这个类加一些行为。
动静分离，有的参数每次函数调用值都不同（动数据），有的参数每次请求值都一样（静数据），变化频率不一致。静态不变的数据完全可以成为这个函数所在类的一个字段。这个坏味道其实是一个软件设计问题，代码缺乏应有的结构，所以，原本应该属于静态结构的部分却以动态参数的方式传来传去，无形之中拉长了参数列表。
解决标记参数，将标记参数代表的不同路径拆分出来。

滥用控制语句

极端的说：代码里只能有一层缩进。如果分支还是很多，可以考虑多态来改进代码
重复的 switch（同一个对象swith不同的字段或逻辑）。之所以会出现重复的 switch，通常都是缺少了一个模型。应对这种坏味道，重构的手法是：以多态取代条件表达式（Relace Conditional with Polymorphism）

缺乏封装。封装这件事并不是很多程序员编码习惯的一部分，他们对封装的理解停留在数据结构加算法的层面上。在学习数据结构时，我们所编写的代码都是拿到各种细节直接操作，但那是在做编程练习，并不是工程上的编码方式。一个好的封装是需要基于行为的，我们应该考虑的问题是类应该提供哪些行为，而非简简单单地把数据换一种形式呈现出来。

火车残骸，示例String name = book.getAuthor().getName();如果你想写出上面这段代码，必须得知道作者的姓名是存储在作品的作者字段里的。当你必须得先了解一个类的细节，才能写出代码时，说明这个封装是失败的。你也许会看到某个类接口有一半的函数都委托给其他类，这样就是过度运用委托。
```
 class Book {
     ...
     public String getAuthorName() {
         return this.author.getName();
     }
     ...
 }
 String name = book.getAuthorName();
```
基本类型偏执。下例中，虽然价格本身是用浮点数在存储，但价格和浮点数本身并不是同一个概念，有着不同的行为需求。比如，一般情况下，我们要求商品价格是大于 0 的，但 double 类型本身是没有这种限制的。如果使用 double 作为类型，那我们要在使用的地方都保证价格的正确性，像这样的价格校验就应该是使用的地方到处写。一旦有了这个模型，我们还可以再进一步，比如，如果我们想要让价格在对外呈现时只有两位，在没有 Price 类的时候，这样的逻辑就会散落代码的各处，事实上，代码里很多重复的逻辑就是这样产生的。
```
 public double getEpubPrice(final boolean highQuality, final int chapterSequence) {
     ...
 }
 class Price {
     private long price;
     public Price(final double price) {
         if (price <= 0) {
             throw new IllegalArgumentException("Price should be positive");
         }
         this.price = price;
     }
     public double getDisplayPrice() {
         BigDecimal decimal = new BigDecimal(this.price)；
         return decimal.setScale(2, BigDecimal.ROUND_HALF_UP).doubleValue();
     }
 }
```
封装之所以有难度，主要在于它是一个构建模型的过程，而很多程序员写程序，只是用着极其粗粒度的理解写着完成功能的代码，根本没有构建模型的意识；

数据类：面向对象编程的主要思想是把行为和数据放在同一个代码单元（一个类）中，如果一个类使用广泛，但是自身没什么重要行为，那么整个系统可能遍布处理实例的代码，并出现在很多方法和函数中（有些甚至是重复的），这对系统维护来说简直是噩梦。刚开始创建一个项目或编写一个模块时，先用数据类实现一个类，随着时间的推移，类应该拥有自己的方法，而不是依赖其它类的方法操作该类的实例，脚手架是临时的。

变量初始化最好一次性完成，避免变量声明与赋值分离。在 C 语言诞生的年代，当时计算机能力有限内存小，编译器技术也处于刚刚起步的阶段，把变量放在前面声明出来，有助于减小编译器编写的难度。今天的大多数程序设计语言来说，这个限制早就不存在了。常见坏味道

变量初始化与业务处理混在一起
在与初始化相隔很远的地方才做了真正的赋值。
声明一个集合，然后调用一堆添加的方法，将所需的对象添加进去。可以使用Guava 或者java9 将集合的声明和初始化一步到位。

依赖混乱

依赖混乱：缺少防腐层，会让请求对象传导到业务代码中，造成了业务与外部接口的耦合，也就是业务依赖了一个外部通信协议。一般来说，业务的稳定性要比外部接口高，这种反向的依赖就会让业务一直无法稳定下来，继而在日后带来更多的问题。解决方案自然就是引入一个防腐层，将业务和接口隔离开来。代码应该向着稳定的方向依赖。

为了避免同时面对所有细节，我们需要把程序进行拆分，分解成一个又一个的小模块。但随之而来的问题就是，我们需要把这些拆分出来的模块按照一定的规则重新组装在一起，这就是依赖的缘起。一个模块要依赖另外一个模块完成完整的业务功能，而到底怎么去依赖，这里就很容易产生问题。

@PostMapping("/books")
public NewBookResponse createBook(final NewBookRequest request) {
  boolean result = this.service.createBook(request);
  ...
}

按照一般代码的分层逻辑，一个 Controller调用一个 Service，这符合大多数人的编程习惯，但问题出在传递的参数上。这个 NewBookRequest 的参数类应该属于哪一层，是 Controller 层，还是 service 层呢？

一般来说，既然它是一个请求参数，通常要承载着诸如参数校验和对象转换的职责，按照我们通常的理解，它应该属于 Controller 层。如果这个理解是正确的，问题就来了，它为什么会传递给 service 层呢？按照通常的架构设计原则，service 层属于我们的核心业务，而 Controller 层属于接口。二者相较而言，核心业务的重要程度更高一些，所以，它的稳定程度也应该更高一些。同样的业务，我们可以用 REST 的方式对外提供，也可以用 RPC 的方式对外提供。NewBookRequest 这个本来应该属于接口层的参数，现在成了核心业务的一部分，也就是说，即便将来我们提供了 RPC 的接口，它也要知道 REST 的接口长什么样子，显然，这是有问题的。
那我们假设它属于 service 层呢？正如我们前面所说，一般请求都要承担对象校验和转化的工作。如果说这个类属于 service 层，但它用在了 Controller 的接口上，作为 Controller 的接口，它会承载一些校验和对象转换的角色，而 service 层的参数是不需要关心这些的。还有更关键的一点是，有时候 service 层的参数和 Controller 层的参数并不是严格地一一对应。比如，创建作品时，我们需要一个识别作者身份的用户 ID，而这个参数并不是通过客户端发起的请求参数带过来，而是根据用户登录信息进行识别的。所以，用 service 层的参数做 Controller 层的参数，就存在差异的参数如何处理的问题。

之所以我们这么纠结，一个关键点在于，我们缺少了一个模型。NewBookRequest 之所以弄得如此“里外不是人”，主要就是因为它只能扮演一个层中的模型，所以，我们只要再引入一个模型就可以破解这个问题。对于 Controller 层的请求对象，因为它的主要作用是传输，所以，一般来说，我们约定请求对象的字段主要是基本类型。而 service 的参数对象，因为它已经是核心业务的一部分，就需要全部转化为业务对象。比如同样表示价格，在请求对象中可以是一个 double 类型，而在业务参数对象中应该是 Price 类型。Controller 层就是外部请求和核心业务之间的防腐层。只要理解了这一点，你就能理解这里要多构建出一个业务参数对象的意义了。

class NewBookParameter { ...}
class NewBookRequest {
  public NewBookParameters toNewBookRequest(long userId) {
    ...
  }
}
@PostMapping("/books")
public NewBookResponse createBook(final NewBookRequest request, final Authentication authentication) {
  long userId = getUserIdentity(authentication);
  boolean result = this.service.createBook(request.toNewBookParameter(userId));
  ...
}

在 Java 世界里可以用 ArchUnit 来保证：Controller 层的代码不能被其它层访问，而 Service 层的代码只能由 Controller 层方法访问。

@Test
public void should_follow_arch_rule() {
  JavaClasses clazz = new ClassFileImporter().importPackages("...");
  ArchRule rule = layeredArchitecture()
        .layer("Resource").definedBy("..resource..")
        .layer("Service").definedBy("..service..")
        .whereLayer("Resource").mayNotBeAccessedByAnyLayer()
        .whereLayer("Service").mayOnlyBeAccessedByLayers("Resource");
        
   rule.check(clazz);       
}

李乾坤

坏味道

前言

不好的命名

重复的代码

长函数和大类

依赖混乱

留下评论

猜您还喜欢

从长期回报、Credit Assignment 到 PPO

Code Agent

Long-horizon Agent

next prompt工程——skill