代码之丑
开篇词 | 这一次,我们从“丑”代码出发
案例一
代码
public void approve(final long bookId) {
...
book.setReviewStatus(ReviewStatus.APPROVED);
...
}
逻辑
这是在一个服务类里面写的,它的主要逻辑就是从仓库中找出一个作品,然后,将它的状 态设置为审核通过,再将它存回去。
原因
审核的状态是作品的一个内部状态,为什么服务需要知道它呢?也就是说,这 里通过 setter,将一个类的内部行为暴露了出来,这是一种破坏封装的做法。
改进
public void approve(final long bookId) {
...
book.approve();
...
}
这段代码,完全是因为这里用到了 setter。setter 的出现,是对于封装的破坏,它把一个类内部的实现细节暴露了出来。面向对象的封装,关键点是行为,而使用 setter 多半只是做了数据的聚合,缺少了行为的设计,这段代码改写后的 approve 函数,就是这里缺少的行为。
再扩展一步,setter 通常还意味着变化,一个好的设计应该尽可能追求不变性。所以,setter 也是一个提示符,告诉我们,这 个地方的设计可能有问题。
注意点
代码写得更具可维护性,这是 一个程序员从业余迈向职业的第一步。
代码坏味道自查表
命名
- 命令是否具有业务含义
- 命令是否符合英语语法
函数
- 代码行是否超过()行
- 参数列表是否超过()个
类
- 类的字段是否超过()个
- 类之间的依赖关系是否符合架构原则
语句
- 是否使用for循环
- 是否使用else
- 是否有重复的switch
- 一行代码中是否出现了连续的方法调用
- 代码中是否出现setter
- 变量生命之后是否有立即赋值
- 集合声明之后是否有立即添加元素
- 返回值是否可以使用Optional
缺乏业务含义的命名:如何精准命名?
不精准的命名
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(处理章节),这个函数确实是 在处理章节,但是,这个名字太过宽泛。如果说“将章节的翻译状态改成翻译中”叫做处 理章节,那么“将章节的翻译状态改成翻译完”是不是也叫处理章节呢?“修改章节内 容”是不是也叫处理章节呢?换句话说,如果各种场景都能够叫处理章节,那么处理章节 就是一个过于宽泛的名字,没有错,但不精准。这就是一类典型的命名问题,从表面上看,这个名字是有含义的,但实际上,它并不能有 效地反映这段代码的含义。
注意点
命名过于宽泛,不能精准描述,这是很多代码在命名上存在的严重问题,也是代码难以理解的根源所在。
改进
回到前面那段代码上,如果它不叫“处理章节”,那应该叫什么呢?首先,命名要能够描 述出这段代码在做的事情。这段代码在做的事情就是“将章节修改为翻译中”。那是不是 它就应该叫 changeChapterToTranlsating 呢? 不可否认,相比于“处理章节”,changeChapterToTranlsating 这个名字已经进了一 步,然而,它也不算是一个好名字,因为它更多的是在描述这段代码在做的细节。我们之 所以要将一段代码封装起来,一个重要的原因就是,我们不想知道那么多的细节。如果把 细节平铺开来,那本质上和直接阅读代码细节差别并不大。
所以,一个好的名字应该描述意图,而非细节。 就这段代码而言, 我们为什么要把翻译状态修改成翻译中,这一定是有原因的,也就是意 图。具体到这里的业务,我们把翻译状态修改成翻译中,是因为我们在这里开启了一个翻 译的过程。所以,这段函数应该命名 startTranslation。
public void startTranslation(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);
}
用技术术语命名
代码
List<Book> bookList = service.getBooks();
问题
这段代码却隐藏另外一个典型得不能再典 型的问题:用技术术语命名。
这个 bookList 变量之所以叫 bookList,原因就是它声明的类型是 List。这种命名在代码 中几乎是随处可见的,比如 xxxMap、xxxSet。 这是一种不费脑子的命名方式,但是,这种命名却会带来很多问题,因为它是一种基于实 现细节的命名方式。 我们都知道,编程有一个重要的原则是面向接口编程,这个原则从另外一个角度理解,就 是不要面向实现编程,因为接口是稳定的,而实现是易变的。虽然在大多数人的理解里, 这个原则是针对类型的,但在命名上,我们也应该遵循同样的原则。为什么?我举个例子 你就知道了。
比如,如果我发现,我现在需要的是一个不重复的作品集合,也就是说,我需要把这个变 量的类型从 List 改成 Set。变量类型你一定会改,但变量名你会改吗?这还真不一定,一 旦出现遗忘,就会出现一个奇特的现象,一个叫 bookList 的变量,它的类型是一个 Set。 这样,一个新的混淆就此产生了。 那有什么更好的名字吗?我们需要一个更面向意图的名字。其实,我们在这段代码里真正 要表达的是拿到了一堆书,所以,这个名字可以命名成 books。
改进
List<Book> books = service.getBooks();
也许你发现了,这个名字其实更简单,但从表意的程度上来说,它却是一个更有效的名字。
事实上,在实际的代码 中,技术名词的出现,往往就代表着它缺少了一个应有的模型。
比如,在业务代码里如果直接出现了 Redis:
public Book getByIsbn(String isbn) {
Book cachedBook = redisBookStore.get(isbn);
if (cachedBook != null) {
return cachedBook;
}
Book book = doGetByIsbn(isbn);
redisBookStore.put(isbn, book);
return book;
}
通常来说,这里真正需要的是一个缓存。Redis 是缓存这个模型的一个实现:
public Book getByIsbn(String isbn) {
Book cachedBook = cache.get(isbn);
if (cachedBook != null) {
return cachedBook;
}
Book book = doGetByIsbn(isbn);
cache.put(isbn, book);
return book;
}
再进一步,缓存这个概念其实也是一个技术术语,从某种意义上说,它也不应该出现在业 务代码中。这方面做得比较好的是 Spring。使用 Spring 框架时,如果需要缓存,我们通 常是加上一个 Annotation(注解):
@Cacheable("books")
public Book getByIsbn(String isbn) {
...
}
用业务语言写代码
无论是不精准的命名也好,技术名词也罢,归根结底,体现的是同一个问题:对业务理解 不到位。
从团队的角度看,让每个人根据自己的理解来命名,确实就有可能出现千奇百怪的名字, 所以,一个良好的团队实践是,建立团队的词汇表,让团队成员有信息可以参考。
案例
团队对于业务有了共同理解,我们也许就可以发现一些更高级的坏味道,比如说下面这个 函数声明:
public void approveChapter(long chapterId, long userId) {
...
}
这个函数的意图是,确认章节内容审核通过。这里有一个问题,chapterId 是审核章节的 ID,这个没问题,但 userId 是什么呢?了解了一下背景,我们才知道,之所以这里要有一 个 userId,是因为这里需要记录一下审核人的信息,这个 userId 就是审核人的 userId。 你看,通过业务的分析,我们会发现,这个 userId 并不是一个好的命名,因为它还需要更 多的解释,更好的命名是 reviewerUserId,之所以起这个名字,因为这个用户在这个场景 下扮演的角色是审核人(Reviewer)。
public void approveChapter(long chapterId, long reviewerUserId) {
...
}
思考
从某种意义上来说,这个坏味道也是一种不精准的命名,但它不是那种一眼可见的坏味 道,而是需要在业务层面上再进行讨论,所以,它是一种更高级的坏味道。
好的命名,是体现业务含义的命名。
总结
坏味道:缺乏业务含义的命名
错误命令
- 宽泛的命令
- 用技术术语命令
命名遵循的原则
- 描述意图,而非细节。
- 面向接口编程,接口是稳定的,实现是易变的。
- 命名中出现技术名词,往往是它缺少了一个模型。
- 使用业务语言。
记住一句话
好的命名,是体现业务含义的命名。
乱用英语:站在中国人的视角来看英文命名
原则:
最低限度的要求是写出来的代码要像是在用 英语表达。
或许你听说过,甚至接触过国内的一些程序员用汉语拼音写代码,这就是一种典型的坏味 道。鉴于现在的一些程序设计语言已经支持了 UTF-8 的编码格式,用汉语拼音写代码,还 不如用汉字直接写代码。
违反语法规则的命名
代码
public void completedTranslate(final List<ChapterId> chapterIds) {
List<Chapter> chapters = repository.findByChapterIdIn(chapterIds);
chapters.forEach(Chapter::completedTranslate);
repository.saveAll(chapters);
}
问题
初看之下,这段代码写得还不错,它要做的是将一些章节的信息标记为翻译完成。
因为 completedTranslate 并不是一个正常的英语函数名。从这个名字你能看出,作者想 表达的是“完成翻译”,因为是已经翻译完了,所以,他用了完成时的 completed,而翻 译是 translate。这个函数名就成了 completedTranslate。
一般来说,常见的命名规则是:类名是一个名词,表示一个对象,而方法名则是一个动词,或者是动宾短语,表示一个动作。
改进
以此为标准衡量这个名字,completedTranslate 并不是一个有效的动宾结构。如果把这个 名字改成动宾结构,只要把“完成”译为 complete,“翻译”用成它的名词形式 translation 就可以了。所以,这个函数名可以改成 completeTranslation。
public void completeTranslation(final List<ChapterId> chapterIds) {
List<Chapter> chapters = repository.findByChapterIdIn(chapterIds);
chapters.forEach(Chapter::completeTranslation);
repository.saveAll(chapters);
}
比如,一个函数名 是 retranslation,其表达的意图是重新翻译,但作为函数名,它应该是一个动词,所以, 正确的命名应该是 retranslate。
不准确的英语词汇
有一次,我们要实现一个章节审核的功能,一个同事先定义出了审核的状态:
public enum ChapterAuditStatus {
PENDING,
APPROVED,
REJECTED;
}
问题
抛开前缀不看,同样是审核,一个用了 audit,一个用了 review。
把 audit 和 review 同时放到了搜索引擎里查了一下。原来,audit 会有更官方的 味道,更合适的翻译应该是审计,而 review 则有更多核查的意思,二者相比,review 更 适合这里的场景。
改进
public enum ChapterReviewStatus {
PENDING,
APPROVED,
REJECTED;
}
在这种情况下,最好的解决方案还是建立起一个业务词汇表,千万不要臆想。一般情况 下,我们都可以去和业务方谈,共同确定一个词汇表,包含业务术语的中英文表达。这样 在写代码的时候,你就可以参考这个词汇表给变量和函数命名。
英语单词的拼写错误
public class QuerySort {
private final SortBy sortBy;
private final SortFiled sortFiled;
...
}
原因
偶尔的拼写错误是不可避免的,这就像我们写文章的时候,出现错别字也是难免 的。之所以要在这个专栏中把拼写错误作为一种独立的坏味道,是因为在很多国内程序员 写的程序中,见到的拼写错误比例是偏高的。
总结
坏味道:乱用英语
英语使用不到
- 违反语法规则
- 不准确的英语词汇
- 英语单词拼写错误
解决方法
- 指定代码规范
- 建立团队词汇表
- 经常性进行代码评审
记住一句话
编写符合英语语法规则的代码。
词汇网址
海词词典在线词典在线翻译_海量正版权威词典官方网站 (dict.cn)
长函数:为什么你总是不可避免地写出长函数?
多长的函数才算“长”?
对于函数长度容忍度高,这是导致长函数产生的关键点。
如果一个人认为 100 行代码不算长,那在他眼中,很多代码根本就是没有问题的,也就更 谈不上看到更多问题了,这其实是一个观察尺度的问题。
一个好的程序员面对代码库时要有不同尺度的观察能力,看设计时,要能够高屋建瓴,看 代码时,要能细致入微。
看具体代码时,一定要能够看到细微之处。“任务分解”,关键点就是将任务拆解得越小越好,这个观点对代码同 样适用。随着对代码长度容忍度的降低,对代码细节的感知力就会逐渐提升,你才能看到 那些原本所谓细枝末节的地方隐藏的各种问题。
长函数的产生
以性能为由
性能优化不应该是写代码的第一考量。 一方面,一门有活力的程序设计语言本身是不断优化的,无论是编译器,还是运行时,性 能都会越来越好;另一方面,可维护性比性能优化要优先考虑,当性能不足以满足需要 时,我们再来做相应的测量,找到焦点,进行特定的优化。这比在写代码时就考虑所谓性 能要更能锁定焦点,优化才是有意义的。
平铺直叙
除了以性能为由把代码写长,还有一种最常见的原因也会把代码写长,那就是写代码平铺 直叙,把自己想到的一点点罗列出来。
我们可以看到平铺直叙的代码存在的两个典型问题:
问题
把多个业务处理流程放在一个函数里实现;
把不同层面的细节放到一个函数里实现。
这里发送章节和启动翻译是两个过程,显然,这是可以放到两个不同的函数中去实现的, 所以,我们只要做一下提取函数,就可以把这个看似庞大的函数拆开,而拆出来的几个函 数规模都会小很多。
注意点
平铺直叙的代码,一个关键点就是没有把不同的东西分解出来。如果我们用设计的眼光衡 量这段代码,这就是“分离关注点”没有做好,把不同层面的东西混在了一起,既有不同 业务混在一起,也有不同层次的处理混在了一起。关注点越多越好,粒度越小越好。
一次加一点
有时,一段代码一开始的时候并不长,就像下面这段代码,它根据返回的错误进行相应地 错误处理:
if (code == 400 || code == 401) {
// 做一些错误处理
}
然后,新的需求来了,增加了新的错误码,它就变成了这个样子:
if (code == 400 || code == 401 || code == 402) {
// 做一些错误处理
}
你知道,一个有生命力的项目经常会延续很长时间,于是,这段代码有很多次被修改的机 会,日积月累,它就成了让人不忍直视的代码,比如:
if (code == 400 || code == 401 || code == 402 || ...
|| code == 500 || ...
|| ...
|| code == 10000 || ...) {
// 做一些错误处理
}
任何代码都经不起这种无意识的累积,每个人都没做错,但最终的结果很糟糕。对抗这种 逐渐糟糕腐坏的代码,我们需要知道“童子军军规”:
让营地比你来时更干净。 —— 童子军军规
Robert Martin 把它借鉴到了编程领域,简言之,我们应该看看自己对于代码的改动是不 是让原有的代码变得更糟糕了,如果是,那就改进它。但这一切的前提是,你要能看出自 己的代码是不是让原有的代码变得糟糕了,所以,学习代码的坏味道还是很有必要的。
总结
坏味道:长函数
长函数的产生
- 以性能为由。
- 平铺直叙写代码。
- 一次增加一点点代码。
消灭长函数的原则
- 定义好函数长度的标准。
- 做好”分离关注点”。
- 坚守”童子军军规”。
重构手法
提取函数
记住一句话
把函数写短,越短越好
大类:如何避免写出难以理解的大类?
大类还有一种表现形式,类里面有特别多的字段和函数,也许,每个函数都不大,但架不 住数量众多啊,这也足以让这个类在大类中占有一席之地。这一讲,我们就主要来说说这 种形式的大类。
分模块的程序
我先来问你一个问题,为什么不把所有的代码都写到一个文件里?
事实是,把代码写到一个文件里,一方面,相同的功能模块没有办法复用;另一方面,也 是更关键的,把代码都写到一个文件里,其复杂度会超出一个人能够掌握的认知范围。简 言之,一个人理解的东西是有限的,没有人能同时面对所有细节。
人类面对复杂事物给出的解决方案是分而治之。所以,我们看到几乎各种程序设计语言都 有自己的模块划分方案,从最初的按照文件划分,到后来,使用面向对象方案按照类进行 划分,本质上,它们都是一种模块划分的方式。这样,人们面对的就不再是细节,而是模 块,模块的数量显然会比细节数量少,人们的理解成本就降低了
如果一个类 里面的内容太多,它就会超过一个人的理解范畴,顾此失彼就在所难免了。
大类的产生
职责不单一
最容易产生大类的原因在于职责的不单一。我们先来看一段代码:
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;
...
}
这个 User 类拥有着一个大类的典型特征,其中包含着一大堆的字段。面对这样一个类时, 我们要问的第一个问题就是,这个类里的字段都是必需的吗? 我们来稍微仔细地看一下这个类,用户 ID(userId)、姓名(name)、昵称 (nickname) 之类应该是一个用户的基本信息,后面的邮箱(email)、电话号码 (phoneNumber) 也算是和用户相关联的。
再往后看,作者类型(authorType),这里表示作者是签约作者还是普通作者,签约作者 可以设置作品的付费信息,而普通作者不能。后面的字段是作者审核状态 (authorReviewStatus),就是说,作者成为签约作者,需要有一个申请审核的过程,这 个状态就是审核的状态。
再往后,又出现了一个编辑类型(editorType),编辑可以是主编,也可以是小编,他们 的权限是不一样的。 这还不是这个 User 类的全部。但是,即便只看这些内容,也足以让我们发现一些问题了。 首先,普通的用户既不是作者,也不是编辑。作者和编辑这些相关的字段,对普通用户来 说,都是没有意义的。其次,对于那些成为了作者的用户,编辑的信息意义也不大,因为 作者是不能成为编辑的,反之亦然,编辑也不会成为作者,作者信息对成为编辑的用户也 是没有意义的。
很多类之所以巨大,大部分原因都是违反了单一职责原则。而想要破解“大类”的谜 题,关键就是能够把不同的职责拆分开来。
改进
User类
public class User {
private long userId;
private String name;
private String nickname;
private String email;
private String phoneNumber;
...
}
Author
public class Author {
private long userId;
private AuthorType authorType;
private ReviewStatus authorReviewStatus;
...
}
Editor
public class Editor {
private long userId;
private EditorType editorType;
}
这里,我们拆分出了 Author 和 Editor 两个类,把与作者和编辑相关的字段分别移到了这 两个类里面。在这两个类里面分别有一个 userId 字段,用以识别这个角色是和哪个用户相 关。这个大 User 类就这样被分解了。
字段未分组
大类的产生往往还有一个常见的原因,就是字段未分组。 有时候,我们会觉得有一些字段确实都是属于某个类,结果就是,这个类还是很大。比 如,我们看一下上面拆分的结果,那个新的 User 类:
public class User {
private long userId;
private String name;
private String nickname;
private String email;
private String phoneNumber;
...
}
前面我们分析过,这些字段应该都算用户信息的一部分。但是,即便相比于原来的 User 类 小了许多,这个类依然也不算是一个小类,原因就是,这个类里面的字段并不属于同一种 类型的信息。比如,userId、name、nickname 几项,算是用户的基本信息,而 email、 phoneNumber 这些则属于用户的联系方式。 从需求上看,基本信息是那种一旦确定就不怎么会改变的内容,而联系方式则会根据实际 情况调整,比如,绑定各种社交媒体的账号。所以,如果我们把这些信息都放到一个类里 面,这个类的稳定程度就要差一些。所以,我们可以根据这个理解,把 User 类的字段分个 组,把不同的信息放到不同的类里面。
改进
从需求上看,基本信息是那种一旦确定就不怎么会改变的内容,而联系方式则会根据实际 情况调整,比如,绑定各种社交媒体的账号。所以,如果我们把这些信息都放到一个类里 面,这个类的稳定程度就要差一些。所以,我们可以根据这个理解,把 User 类的字段分个 组,把不同的信息放到不同的类里面。
代码
public class User {
private long userId;
private String name;
private String nickname;
private Contact contact;
...
}
public class Contact {
private String email;
private String phoneNumber;
...
}
这里我们引入了一个 Contact 类(也就是联系方式),把 email 和 phoneNumber 放了 进去,后面再有任何关于联系方式的调整就都可以放在这个类里面。经过这次调整,我们 把不同的信息重新组合了一下,但每个类都比原来要小。
或许你已经发现了,所谓的将大类拆解成小类,本质上在做的工作是一个设计工作。
总结
坏味道:大类
产生大类的原因
- 职责不单一
- 字段未分组
软件设计的原则
单一职责原则
极致的追求
每个类不超过2个字段
记住一句话
把类写小,越小越好
长参数列表:如何处理不同类型的长参数?
那么,函数为什么要有参数呢?我们知道,不同函数之间需要共享信息,于是才有了参数传递。
函数之间还是要传递信息的,既然不能用全局变量,参数就成了最好的选择,于是乎, 只要你想到有什么信息要传给一个函数,就自然而然地把它加到参数列表中,参数列表也 就越来越长了。 那么,长参数列表有啥问题呢?这个问题其实我在上一讲已经说过了,人脑能够掌握的内 容有限,一旦参数列表变得很长,作为普通人,我们就很难对这些内容进行把控了。 既然长参数列表的问题是数量多,秉承我们一以贯之的思路,解决这个问题的关键就在 于,减少参数的数量。
聚沙成塔
代码
public void createBook(final String title,
final String introduction,
final URL coverUrl,
final BookType type,
final BookChannel channel,
final String protagonists,
final String tags,
final boolean completed) {
...
Book book = Book.builder
.title(title)
.introduction(introduction)
.coverUrl(coverUrl)
.type(type)
.channel(channel)
.protagonists(protagonists)
.tags(tags)
.completed(completed)
.build();
this.repository.save(book);
}
这里所有的参数其实都是和作品相关的,也就是说,所有的参数都是创建作品所必需的。 所以,我们可以做的就是将这些参数封装成一个类,一个创建作品的参数类:
改进
public class NewBookParamters {
private String title;
private String introduction;
private URL coverUrl;
private BookType type;
private BookChannel channel;
private String protagonists;
private String tags;
private boolean completed;
...
}
这样一来,这个函数参数列表就只剩下一个参数了,一个长参数列表就消除了:
public void createBook(final NewBookParamters parameters) {
...
}
这里你看到了一个典型的消除长参数列表的重构手法:将参数列表封装成对象。 或许你还有个疑问,只是把一个参数列表封装成一个类,然后,用到这些参数的时候,还 需要把它们一个个取出来,这会不会是多此一举呢?就像这样:
public void createBook(final NewBookParamters parameters) {
...
Book book = Book.builder
.title(parameters.getTitle())
.introduction(parameters.getIntroduction())
.coverUrl(parameters.getCoverUrl())
.type(parameters.getType())
.channel(parameters.getChannel())
.protagonists(parameters.getProtagonists())
.tags(parameters.getTags())
.completed(parameters.isCompleted())
.build();
this.repository.save(book);
}
一个模型的封装应该是以行为为基础的。
之前没有这个模型,所以,我们想不到它应该有什么行为,现在模型产生了,它就应该有 自己配套的行为,那这个模型的行为是什么呢?从上面的代码我们不难看出,它的行为应 该是构建一个作品对象出来。你理解了这一点,我们的代码就可以进一步调整了:
public class NewBookParamters {
private String title;
private String introduction;
private URL coverUrl;
private BookType type;
private BookChannel channel;
private String protagonists;
private String tags;
private boolean completed;
public Book newBook() {
return Book.builder
.title(title)
.introduction(introduction)
.coverUrl(coverUrl)
.type(type)
.channel(channel)
.protagonists(protagonists)
.tags(tags)
.completed(completed)
.build();
}
}
创建作品的函数就得到了极大的简化:
public void createBook(final NewBookParamters parameters) {
...
Book book = parameters.newBook();
this.repository.save(book);
}
动静分离
把长参数列表封装成一个类,这能解决大部分的长参数列表,但并不等于所有的长参数列 表都应该用这种方式解决,因为不是所有情况下,参数都属于一个类。
代码
public void getChapters(final long bookId,
final HttpClient httpClient,
final ChapterProcessor processor) {
HttpUriRequest request = createChapterRequest(bookId);
HttpResponse response = httpClient.execute(request);
List<Chapter> chapters = toChapters(response);
processor.process(chapters);
}
这个函数的作用是根据作品 ID 获取其对应的章节信息。如果,单纯以参数个数论,这个函 数的参数数量并不算多。 如果你只是看这个函数,可能很难发现直接的问题。即便我们认为有问题,也可以用一个 类把这个函数的参数都封装起来。
改进
不同的数据变动方向也是不同的 关注点。这里表现出来的就是典型的动数据(bookId)和静数据(httpClient 和 processor),它们是不同的关注点,应该分离开来。具体到这个场景下,静态不变的数据完全可以成为这个函数所在类的一个字段,而只将每 次变动的东西作为参数传递就可以了。按照这个思路,代码可以改成这个样子:
public void getChapters(final long bookId) {
HttpUriRequest request = createChapterRequest(bookId);
HttpResponse response = this.httpClient.execute(request);
List<Chapter> chapters = toChapters(response);
this.processor.process(chapters);
}
这个例子也给了我们一个提示,长参数列表固然可以用一个类进行封装,但能够封装出这 个类的前提条件是:这些参数属于一个类,有相同的变化原因。
告别标记
public void editChapter(final long chapterId,
final String title,
final String content,
final boolean apporved) {
...
}
之所以要有这么个参数,从业务上说,如果是作者进行编辑,之后要经过审核,而如果编 辑来编辑的,那审核就直接通过,因为编辑本身扮演了审核人的角色。所以,你发现了, 这个参数实际上是一个标记,标志着接下来的处理流程会有不同。 使用标记参数,是程序员初学编程时常用的一种手法,不过,正是因为这种手法实在是太 好用了,造成的结果就是代码里面彩旗(flag)飘飘,各种标记满天飞。不仅变量里有标 记,参数里也有。很多长参数列表其中就包含了各种标记参数。这也是很多代码产生混乱 的一个重要原因。 在实际的代码中,我们必须小心翼翼地判断各个标记当前的值,才能做好处理。 解决标记参数,一种简单的方式就是,将标记参数代表的不同路径拆分出来。回到这段代 码上,这里的一个函数可以拆分成两个函数,一个函数负责“普通的编辑”,另一个负 责“可以直接审核通过的编辑”。
// 普通的编辑,需要审核
public void editChapter(final long chapterId,final String title,final String content) {
...
}
// 直接审核通过的编辑
public void editChapterWithApproval(final long chapterId,final String title,final String content) {
...
}
标记参数在代码中存在的形式很多,有的是布尔值的形式,有的是以枚举值的形式,还有 的就是直接的字符串或者整数。无论哪种形式,我们都可以通过拆分函数的方式将它们拆 开。在重构中,这种手法叫做移除标记参数(Remove Flag Argument)。
总结
坏味道:长参数
消除长参数
- 参数数量多导致的长参数
- 变化频率相同,则封装成一个类。
- 变化频率不同:
- 静态不变的,可以成为软件结构的一部分;
- 多个变化频率的,可以封装成几个类。
- 标记参数导致的长函数
- 根据标记参数,将函数拆分多个函数。
重构的手法
- 将参数列表封装成对象。
- 移除标记参数(Remove Flag Argument)。
记住一句话
减小参数列表,越小越好。
滥用控制语句:出现控制结构,多半是错误的提示
嵌套的代码
我们还是从规模小一点的代码开始讨 论:
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);
}
}
}
}
这是一段做 EPUB 分发的代码,EPUB 是一种电子书格式。在这里,我们根据作品 ID 找到 要分发的 EPUB,然后检查 EPUB 的有效性。对于有效的 EPUB,我们要为它注册 ISBN 信 息,注册成功之后,将这个 EPUB 发送出去。 代码逻辑并不是特别复杂,只不过,在这段代码中,我们看到了多层的缩进,for 循环一 层,里面有两个 if ,又多加了两层。即便不是特别复杂的代码,也有这么多的缩进,可想 而知,如果逻辑再复杂一点,缩进会成什么样子。 这段代码之所以会写成这个样子,其实就是我在讲“长函数”那节课里所说的:“平铺 直叙地写代码”。这段代码的作者只是按照需求一步一步地把代码实现出来了。从实现功 能的角度来说,这段代码肯定没错,但问题在于,在把功能实现之后,他停了下来,而没 有把代码重新整理一下。那我们就来替这段代码作者将它整理成应有的样子。
改进
public void distributeEpubs(final long bookId) {
List<Epub> epubs = this.getEpubsByBookId(bookId);
for (Epub epub : epubs) {
this.distributeEpub(epub);
}
}
private void distributeEpub(final Epub epub) {
if (epub.isValid()) {
boolean registered = this.registerIsbn(epub);
if (registered) {
this.sendEpub(epub);
}
}
}
这里我们已经有了一次拆分,分解出来 distributeEpub 函数每次只处理一个元素。拆分出 来的两个函数在缩进的问题上,就改善了一点。 第一个函数 distributeEpubs 只有一层缩进,这是一个正常函数应有的样子,不过,第二 个函数 distributeEpub 则还有多层缩进,我们可以继续处理一下。
if 和 else
在 distributeEpub 里,造成缩进的原因是 if 语句。通常来说,if 语句造成的缩进,很多时 候都是在检查某个先决条件,只有条件通过时,才继续执行后续的代码。这样的代码可以 使用卫语句(guard clause)来解决,也就是设置单独的检查条件,不满足这个检查条件 时,立刻从函数中返回。 这是一种典型的重构手法:以卫语句取代嵌套的条件表达式(Replace Nested Conditional with Guard Clauses)。 我们来看看改进后的 distributeEpub 函数:
private void distributeEpub(final Epub epub) {
if (!epub.isValid()) {
return;
}
boolean registered = this.registerIsbn(epub);
if (!registered) {
return;
}
this.sendEpub(epub);
}
改造后的 distributeEpub 就没有了嵌套,也就没有那么多层的缩进了。你可能已经发现 了,经过我们改造之后,代码里只有一层的缩进。当代码里只有一层缩进时,代码的复杂 度就大大降低了,理解成本和出现问题之后定位的成本也随之大幅度降低。 函数至多有一层缩进,这是“对象健身操(《ThoughtWorks 文集》书里的一篇)”里 的一个规则。前面讲“大类”的时候,我曾经提到过“对象健身操”这篇文章,其中给出了九条编程规则,下面我们再来讲其中的一条:不要使用 else 关键字。 没错,else 也是一种坏味道,这是挑战很多程序员认知的。在大多数人印象中,if 和 else 是亲如一家的整体,它们几乎是比翼齐飞的。那么,else 可以不写吗?可以。我们来看看 下面的代码:
public double getEpubPrice(final boolean highQuality, final int chapterSequenc){
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;
}
就这段代码而言,如果想不使用 else,一个简单的处理手法就是让每个逻辑提前返回,这 和我们前面提到的卫语句的解决方案如出一辙:
public double getEpubPrice(final boolean highQuality, final int chapterSequenc){
if (highQuality && chapterSequence > START_CHARGING_SEQUENCE) {
return 4.99;
}
if (sequenceNumber > START_CHARGING_SEQUENCE && sequenceNumber <= FURTHER_CHARGING_SEQUENCE) {
return 1.99;
}
if (sequenceNumber > FURTHER_CHARGING_SEQUENCE) {
return 2.99;
}
return 0.99;
}
无论是嵌套的代码,还是 else 语句,我们之所以要把它们视为坏味道,本质上都在追求简 单,因为一段代码的分支过多,其复杂度就会大幅度增加。我们一直在说,人脑能够理解 的复杂度是有限的,分支过多的代码一定是会超过这个理解范围。
在软件开发中,有一个衡量代码复杂度常用的标准,叫做圈复杂度(Cyclomatic complexity,简称 CC),圈复杂度越高,代码越复杂,理解和维护的成本就越高。在圈 复杂度的判定中,循环和选择语句占有重要的地位。圈复杂度可以使用工具来检查,比 如,在 Java 世界中,有很多可以检查圈复杂度的工具,我们之前提到过的 Checkstyle 就 可以做圈复杂度的检查,你可以限制最大的圈复杂度,当圈复杂度大于某个值的时候, 就会报错。
重复的 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;
}
}
public double getEpubPrice(final User user, final Epub epub) {
double price = epub.getPrice();
switch (user.getLevel()) {
case UserLevel.SILVER:
return price * 0.95;
case UserLevel.GOLD:
return price * 0.85;
case UserLevel.PLATINUM:
return price * 0.8;
default:
return price;
}
}
然,这两个函数里出现了类似的代码,其中最类似的部分就是 switch,都是根据用户级 别进行判断。事实上,这并不是仅有的根据用户级别进行判断的代码,各种需要区分用户 级别的场景中都有类似的代码,而这也是一种典型的坏味道:重复的 switch(Repeated Switch)。 之所以会出现重复的 switch,通常都是缺少了一个模型。所以,应对这种坏味道,重构的 手法是:以多态取代条件表达式(Relace Conditional with Polymorphism)。具体到 这里的代码,我们可以引入一个 UserLevel 的模型,将 switch 消除掉。
interface UserLevel {
double getBookPrice(Book book);
double getEpubPrice(Epub epub);
}
class RegularUserLevel implements UserLevel {
public double getBookPrice(final Book book) {
return book.getPrice();
}
public double getEpubPrice(final Epub epub) {
return epub.getPrice();
}
}
class GoldUserLevel implements UserLevel {
public double getBookPrice(final Book book) {
return book.getPrice() * 0.8;
}
public double getEpubPrice(final Epub epub) {
return epub.getPrice() * 0.85;
}
}
class SilverUserLevel implements UserLevel {
public double getBookPrice(final Book book) {
return book.getPrice() * 0.9;
}
public double getEpubPrice(final Epub epub) {
return epub.getPrice() * 0.85;
}
}
class PlatinumUserLevel implements UserLevel {
public double getBookPrice(final Book book) {
return book.getPrice() * 0.75;
}
public double getEpubPrice(final Epub epub) {
return epub.getPrice() * 0.8;
}
}
有了这个基础,前面的代码就可以把 switch 去掉了:
public double getBookPrice(final User user, final Book book) {
UserLevel level = user.getUserLevel()
return level.getBookPrice(book);
}
public double getEpubPrice(final User user, final Epub epub) {
UserLevel level = user.getUserLevel()
return level.getEpubPrice(epub);
}
总结
坏味道:滥用控制结构
坏味道呈现形态
- 嵌套的代码。
- else语句。
- 重复的switch。
- 循环语句。
编程规则
- 函数至多有一层缩进。
- 不要使用else关键字。
重构的手法
- 以卫语句取代嵌套的条件表达式。
- 多态取代条件表达式。
记住一句话
循环和选择语句,可能都是坏味道。
缺乏封装:如何应对火车代码和基本类型偏执问题?
在程序设计中,一个重要的观念就是封装,将零散的代码封装成一个又一个可复用的模 块。任何一个程序员都会认同封装的价值,但是,具体到写代码时,每个人对于封装的理 解程度却天差地别,造成的结果就是:写代码的人认为自己提供了封装,但实际上,我们 还是看到许多的代码散落在那里。
火车残骸
String name = book.getAuthor().getName();
这段代码表达的是“获得一部作品作者的名字”。作品里有作者信息,想要获得作者的名 字,通过“作者”找到“作者姓名”,这就是很多人凭借直觉写出的代码,不过它是有问 题的。 如果你没看出这段代码的问题,说明你可能对封装缺乏理解。 你可以想一想,如果你想写出上面这段代码,是不是必须得先了解 Book 和 Author 这两 个类的实现细节?也就是说,我们必须得知道,作者的姓名是存储在作品的作者字段里 的。这时你就要注意了:当你必须得先了解一个类的细节,才能写出代码时,这只能说明 一件事,这个封装是失败的。
Martin Fowler 在《重构》中给这种坏味道起的名字叫过长的消息链(Message Chains),而有人则给它起了一个更为夸张的名字:火车残骸(Train Wreck),形容 这样的代码像火车残骸一般,断得一节一节的。 解决这种代码的重构手法叫隐藏委托关系(Hide Delegate),说得更直白一些就是,把 这种调用封装起来:
class Book {
...
public String getAuthorName() {
return this.author.getName();
}
...
}
String name = book.getAuthorName();
要想摆脱初级程序员的水平,就要先从少暴露细节开始。声明完一个类的字段之后,请停 下生成 getter 的手,转而让大脑开始工作,思考这个类应该提供的行为。 在软件行业中,有一个编程的指导原则几乎就是针对这个坏味道的,叫做迪米特法则 (Law of Demeter),这个原则是这样说的:
每个单元对其它单元只拥有有限的知识,而且这些单元是与当前单元有紧密联系的;
每个单元只能与其朋友交谈,不与陌生人交谈;
只与自己最直接的朋友交谈。
基本类型偏执
public double getEpubPrice(final boolean highQuality, final int chapterSequenc
...
}
这是我们上一讲用过的一个函数声明,根据章节信息获取 EPUB(一种电子书的格式) 的价格。也许你会问,这是一个看上去非常清晰的代码,难道这里也有坏味道吗? 没错,有。问题就出在返回值的类型上,也就是价格的类型上。 那么,我们在数据库中存储价格的时候,就是用一个浮点数,这里用 double 可以保证计算的精度,这样的设计有什么问题吗? 确实,这就是很多人使用基本类型(Primitive)作为变量类型思考的角度。但实际上,这种采用基本类型的设计缺少了一个模型。 虽然价格本身是用浮点数在存储,但价格和浮点数本身并不是同一个概念,有着不同的行 为需求。比如,一般情况下,我们要求商品价格是大于 0 的,但 double 类型本身是没有 这种限制的。 就以“价格大于 0”这个需求为例,如果使用 double 类型你会怎么限制呢?我们通常会 这样写:
if (price <= 0) {
throw new IllegalArgumentException("Price should be positive");
}
问题是,如果使用 double 作为类型,那我们要在使用的地方都保证价格的正确性,像这 样的价格校验就应该是使用的地方到处写的。 如果补齐这里缺失的模型,我们可以引入一个 Price 类型,这样的校验就可以放在初始化 时进行:
class Price {
private long price;
public Price(final double price) {
if (price <= 0) {
throw new IllegalArgumentException("Price should be positive");
}
this.price = price;
}
}
这种引入一个模型封装基本类型的重构手法,叫做以对象取代基本类型(Replace Primitive with Object)。一旦有了这个模型,我们还可以再进一步,比如,如果我们想 要让价格在对外呈现时只有两位,在没有 Price 类的时候,这样的逻辑就会散落代码的各 处,事实上,代码里很多重复的逻辑就是这样产生的。而现在我们可以在 Price 类里提供 一个方法:
public double getDisplayPrice() {
BigDecimal decimal = new BigDecimal(this.price);
return decimal.setScale(2, BigDecimal.ROUND_HALF_UP).doubleValue();
}
其实,使用基本类型和使用继承出现的问题是异曲同工的。大部分程序员都学过这样一个 设计原则:组合优于继承,也就是说,我们不要写出这样的代码:
public Books extends List<Book> {
...
}
而应该写成组合的样子,也就是:
public Books {
private List<Book> books;
...
}
之所以有人把 Books 写成了继承,因为在代码作者眼中,Books 就是一个书的集合;而有 人用 double 做价格的类型,因为在他看来,价格就是一个 double。这里的误区就在于, 一些程序员只看到了模型的相同之处,却忽略了差异的地方。Books 可能不需要提供 List 的所有方法,价格的取值范围与 double 也有所差异。 但是,Books 的问题相对来说容易规避,因为产生了一个新的模型,有通用的设计原则帮 助我们判断这个模型构建得是否恰当,而价格的问题却不容易规避,因为这里没有产生新 的模型,也就不容易发现这里潜藏着问题。 这种以基本类型为模型的坏味道称为基本类型偏执(Primitive Obsession)。这里说的基 本类型,不限于程序设计语言提供的各种基本类型,像字符串也是一个产生这种坏味道的 地方。
封装之所以有难度,主要在于它是一个构建模型的过程,而很多程序员写程序,只是用着 极其粗粒度的理解写着完成功能的代码,根本没有构建模型的意识;还有一些人以为划分 了模块就叫封装,所以,我们才会看到这些坏味道的滋生。
总结
坏味道:缺乏封装
坏味道呈现形态
- 火车残骸/过长的消息链。
- 基本类型偏执。
编程规则
- 遵循迪米特法则。
- 封装所有的基本类型和字符串。
- 使用一流的集合。
重构的手法
- 隐藏委托关系。
- 以对象取代基本类型。
记住一句话
构建模型,封装散落的代码。
可变的数据:不要让你的代码“失控”
可变的数据是可怕,但是,比可变的数据更可怕的是,不可控的变化,而暴露 setter 就是 这种不可控的变化。把各种实现细节完全交给对这个类不了解的使用者去修改,没有人会 知道他会怎么改,所以,这种修改完全是不可控的。
缺乏封装再加上不可控的变化,在我个人心目中,setter 几乎是排名第一的坏味道。
消除 setter ,有一种专门的重构手法,叫做移除设值函数(Remove Setting Method)。总而言之,setter 是完全没有必要存在的。
可变的数据
我们反对使用 setter,一个重要的原因就是它暴露了数据,我们前面说过,暴露数据造成 的问题就在于数据的修改,进而导致出现难以预料的 Bug。在上面的代码中,我们把 setter 封装成一个个的函数,实际上是把不可控的修改限制在一个有限的范围内。
那么,这个思路再进一步的话,如果我们的数据压根不让修改,犯下各种低级错误的机会 就进一步降低了。没错,在这种思路下,可变数据(Mutable Data)就成了一种坏味 道,这是 Martin Fowler 在新版《重构》里增加的坏味道,它反映着整个行业对于编程 的新理解。
这种想法源自函数式编程这种编程范式。在函数式编程中,数据是建立在不改变的基础上 的,如果需要更新,就产生一份新的数据副本,而旧有的数据保持不变。随着函数式编程 在软件开发领域中的地位不断提高,人们对于不变性的理解也越发深刻,不变性有效地解 决了可变数据产生的各种问题。
解决可变数据,还有一个解决方案是编写不变类
其中的关键点就是设计不 变类。Java 中的 String 类就是一个不变类,比如,如果我们把字符串中的一个字符替换成 另一个字符,String 类给出的函数签名是这样的:
String replace(char oldChar, char newChar);
其含义是,这里的替换并不是在原有字符串上进行修改,而是产生了一个新的字符串。
那么,在实际工作中,我们怎么设计不变类呢?要做到以下三点:
- 所有的字段只在构造函数中初始化;
- 所有的方法都是纯函数;
- 如果需要有改变,返回一个新的对象,而不是修改已有字段。
回过头来看我们之前改动的“用构造函数消除 setter”的代码,其实就是朝着这个方向在 迈进。如果按照这个思路改造我们前面提到的 approve 函数,同样也可以:
class Book {
public void approve() {
return new Book(..., ReviewStatus.APPROVED, ...);
}
}
在 JDK 的演化中,我们可以看到一个很明显的趋势,新增的类越来越多地采用了不变类的 设计,比如,用来表示时间的类。原来的 Date 类里面还有各种 setter,而新增的 LocalDateTime 则一旦初始化就不会再修改了。如果要操作这个对象,则会产生一个新的 对象:
LocalDateTime twoDaysLater = now.plusDays(2);
我们最核心要识别的对象分成两种,实体和值对象。实体对象要限制数据变化,而 值对象就要设计成不变类。
如果你还想进一步提升自己对于不变性的理解,我们可以回到函数式编程这个编程范式的 本质,它其实是对程序中的赋值进行了约束。基于这样的理解,连赋值本身其实都会被归 入到坏味道的提示,这才是真正挑战很多人编程习惯的一点。
总结
坏味道:可变的数据
坏味道呈现形态
- 暴露的细节。
- 可变的数据。
- 全局数据。
编程规则
- 限制变化。
- 尽可能编写不变类。
- 区分类的性质,实体对象要限制数据变化,而值对象就要设计成不变类。
重构的手法
移除设置函数
记住一句话
限制可变的数据。
变量声明与赋值分离:普通的变量声明,怎么也有坏味道?
变量的初始化
EpubStatus status = null;
CreateEpubResponse response = createEpub(request);
if (response.getCode() == 201) {
status = EpubStatus.CREATED;
} else {
status = EpubStatus.TO_CREATE;
}
这段代码在做的事情是向另外一个服务发请求创建 EPUB(一种电子书格式),如果创建 成功,返回值是 HTTP 的 201,也就表示创建成功,然后就把状态置为 CREATED;而如 果没有成功,则把状态置为 TO_CREATE。后面对于 TO_CREATE 状态的作品,还需要再 次尝试创建。
我们这次的重点在 status 这个变量上,虽然 status 这个变量在声明的时候,就赋上了一 个 null 值,但实际上,这个值并没有起到任何作用,因为 status 的变量值,其实是在经过 后续处理之后,才有了真正的值。换言之,从语义上说,第一行的变量初始化其实是没有 用的,这是一次假的初始化。 按照我们通常的理解,一个变量的初始化是分成了声明和赋值两个部分,而我这里要说的 就是,变量初始化最好一次性完成。这段代码里的变量赋值是在声明很久之后才完成的, 也就是说,变量初始化没有一次性完成。 这种代码真正的问题就是不清晰,变量初始化与业务处理混在在一起。通常来说,这种代 码后面紧接着就是一大堆更复杂的业务处理。当代码混在一起的时候,我们必须小心翼翼 地从一堆业务逻辑里抽丝剥茧,才能把逻辑理清,知道变量到底是怎么初始化的。很多代 码难读,一个重要的原因就是把不同层面的代码混在了一起。
所以,我们编程时要有一个基本原则:变量一次性完成初始化。
改进
final CreateEpubResponse response = createEpub(request);
final EpubStatus status = toEpubStatus(response);
private EpubStatus toEpubStatus(final CreateEpubResponse response) {
if (response.getCode() == 201) {
return EpubStatus.CREATED;
}
return EpubStatus.TO_CREATE;
}
还有一点不知道你注意到了没有,在新的变量声明中,我加上了 final,在 Java 的语义 中,一个变量加上了 final,也就意味着这个变量不能再次赋值。对,我们需要的正是这样 的限制。 上一讲,我们讲了可变的数据会带来怎样的影响,其中的一个结论是,尽可能编写不变的 代码。这里其实是这个话题的延伸,尽可能使用不变的量。
思考
在能够使用 final 的地方尽量使用 final,限制变量的赋值。
这里说的“能够使用”,不仅包括普通的变量声明,还包含参数声明,还有类字段的声 明,甚至还可以包括类和方法的声明。当然,我们这里改进的考量主要还是在变量上。
例子
InputStream is = null;
try {
is = new FileInputStream(...);
...
} catch (IOException e) {
...
} finally {
if (is != null) {
is.close();
}
}
之所以要把 InputStream 变量 is 单独声明,是为了能够在 finanlly 块里面访问到。其 实,这段代码写成这样,一个重要的原因是 Java 早期的版本只能写成这样,而如果采用 Java 7 之后的版本,采用 try-with-resource 的写法,代码就可以更简洁了:
try (InputStream is = new FileInputStream(...)) {
...
}
这样一来,InputStream 变量的初始化就一次性完成了,我们的原则就统一了,不需要在 这种特殊的场景下纠结了。
集合初始化
接下来,我们在来看一段代码:
List<Permission> permissions = new ArrayList<>();
permissions.add(Permission.BOOK_READ);
permissions.add(Permission.BOOK_WRITE);
check.grantTo(Role.AUTHOR, permissions);
这是一段给作者赋予作品读写权限的代码,逻辑比较简单,但这段代码中也存在一些坏味 道。我们把注意力放在 permissions 这个集合上。之所以要声明这样一个 List,是因为 grantTo 方法要用到一个 List 作为参数。 我们来看这个 List 是怎样生成的。这里先给 permission 初始化成了一个 ArrayList,这个 时候,permissions 虽然存在了,但我们并不会把它传给 grantTo 方法,它还不能直接使 用,因为它还缺少必要的信息。然后,我们将 BOOK_READ 和 BOOK_WRITE 两个枚举对 象添加了进去,这样,这个 permissions 对象才是我们真正需要的那个对象。
我们不难发现,其实 permissions 对象一开始的变量声明,并没有完成这个集合真正的初 始化,只有当集合所需的对象添加完毕之后,这个集合才是它应有的样子。换言之,只有 添加了元素的集合才是我们需要的。
改进
这样解释这段代码,你是不是就发现了,这和我们前面所说的变量先声明后赋值,本质上 是一回事,都是从一个变量的声明到初始化成一个可用的状态,中间隔了太远的距离。 之所以很多人习惯这么写,一个原因就是在早期的 Java 版本中,没有提供很好的集合初始化的方法。像这种代码,也是很多动态语言的支持者调侃 Java 啰嗦的一个靶子。
现如今,Java 在这方面早已经改进了许多,各种程序库已经提供了一步到位的写法,我们 先来看看 Java 9 之后的写法:
List<Permission> permissions = List.of(
Permission.BOOK_READ,
Permission.BOOK_WRITE
);
check.grantTo(Role.AUTHOR, permissions);
如果你的项目还没有升级 Java 9 之后的版本,使用 Guava(Google 提供的一个 Java 库)也是可以做成类似的效果:
List<Permission> permissions = ImmutableList.of(
Permission.BOOK_READ,
Permission.BOOK_WRITE
);
check.grantTo(Role.AUTHOR, permissions);
经过改进,这段代码是不是看上去就清爽多了! 不知道你注意到没有,第二段代码里的 List 用的是一个 ImmutableList,也就是一个不可 变的 List,实际上,你查看第一段代码的实现就会发现,它也是一个不变的 List。这是什么 意思呢?也就是说,这个 List 一旦创建好了,就是不能修改了,对应的实现就是各种添 加、删除之类的方法全部都禁用了。 初看起来,这是限制了我们的能力,但我们对比一下代码就不难发现,很多时候,我们对 于一个集合的使用,除了声明时添加元素之外,后续就只是把它当作一个只读的集合。所 以,在很多情况下,一个不变集合对我们来说就够用了。 其实,这段代码,相对来说还是比较清晰的,稍微再复杂一些的,集合的声明和添加元素 之间隔了很远,不注意的话,甚至不觉得它们是在完成一次初始化。
private static Map<Locale, String> CODE_MAPPING = new HashMap<>();
...
static {
CODE_MAPPING.put(LOCALE.ENGLISH, "EN");
CODE_MAPPING.put(LOCALE.CHINESE, "CH");
}
CODE_MAPPING 是一个类的 static 变量,而这个类的声明里还有其它一些变量。所以, 隔了很远之后,才有一个 static 块向这个集合添加元素。 如果我们能够用一次性声明的方式,这个单独的 static 块就是不需要的:
private static Map<Locale, String> CODE_MAPPING = ImmutableMap.of(
LOCALE.ENGLISH, "EN",
LOCALE.CHINESE, "CH"
);
对比我们改造前后的代码,二者之间还有一个更关键的区别:前面的代码是命令式的代 码,而后面的代码是声明式的代码。 命令式的代码,就是告诉你“怎么做”的代码,就像改造前的代码,声明一个集合,然后 添加一个元素,再添加一个元素。而声明式的代码,是告诉你“做什么”的代码,改造后 就是,我要一个包含了这两个元素的集合。
总结
坏味道:变量的声明与赋值分离
编程规则
变量要一次性完成初始化。
应对策略
- 在声明前面加上final,用不变性的限制约束代码。
- 用声明式的方法进行集合的初始化。
记住一句话
一次性完成变量初始化。
依赖混乱:你可能还没发现问题,代码就已经无法挽救了
“大类”这个坏味道的时候曾经说过,为了避免同时面对所有细节,我们需要 把程序进行拆分,分解成一个又一个的小模块。但随之而来的问题就是,我们需要把这些 拆分出来的模块按照一定的规则重新组装在一起,这就是依赖的缘起。
缺少防腐层
案例
代码
@PostMapping("/books")
public NewBookResponse createBook(final NewBookRequest request) {
boolean result = this.service.createBook(request);
...
}
这段代码是创建一部作品的入口,也就是说,它提供了一个 REST 服务,只要我们对 /books 这个地址发出一个 POST 请求,就可以创建一部作品出来。那么,这段代码有问题 吗? 按照一般代码的分层逻辑,一个 Resource (有的团队称之为 Controller)调用一个 Service,这符合大多数人的编程习惯,所以看起来,这段代码简直是正常得不能再正常 了,这能有什么问题? 从 Resource 调用 Service,这几乎是行业里的标准做法,是没有问题的,但问题出在传递 的参数上。请问,这个 NewBookRequest 的参数类应该属于哪一层,是 resource 层,还 是 service 层呢?
一般来说,既然它是一个请求参数,通常要承载着诸如参数校验和对象转换的职责,按照 我们通常的理解,它应该属于 resource 层。如果这个理解是正确的,问题就来了,它为什 么会传递给 service 层呢?
按照通常的架构设计原则,service 层属于我们的核心业务,而 resource 层属于接口。二 者相较而言,核心业务的重要程度更高一些,所以,它的稳定程度也应该更高一些。同样 的业务,我们可以用 REST 的方式对外提供,也可以用 RPC 的方式对外提供。
说到这,你就会发现一个问题,NewBookRequest 这个本来应该属于接口层的参数,现在 成了核心业务的一部分,也就是说,即便将来我们提供了 RPC 的接口,它也要知道 REST的接口长什么样子,显然,这是有问题的。
既然 NewBookRequest 属于 resource 层是有问题的,那我们假设它属于 service 层呢? 正如我们前面所说,一般请求都要承担对象校验和转化的工作。如果说这个类属于 service 层,但它用在了 resource 的接口上,作为 resource 的接口,它会承载一些校验和对象转 换的角色,而 service 层的参数是不需要关心这些的。如果 NewBookRequest 属于 service 层,那校验和对象转换的职责到底由谁来完成呢?
还有更关键的一点是,有时候 service 层的参数和 resource 层的参数并不是严格地一一对 应。比如,创建作品时,我们需要一个识别作者身份的用户 ID,而这个参数并不是通过客 户端发起的请求参数带过来,而是根据用户登录信息进行识别的。所以,用 service 层的 参数做 resource 层的参数,就存在差异的参数如何处理的问题。
你有没有发现,我们突然陷入了一种两难的境地,如此一个简单的参数,放到哪个层里都有问题。
改进
其实,之所以我们这么纠结,一个关键点在于,我们缺少了一个模型。 NewBookRequest 之所以弄得如此“里外不是人”,主要就是因为它只能扮演一个层中的 模型,所以,我们只要再引入一个模型就可以破解这个问题。
class NewBookParameter {
...
}
class NewBookRequest {
public NewBookParameters toNewBookRequest() {
...
}
}
@PostMapping("/books")
public NewBookResponse createBook(final NewBookRequest request) {
boolean result = this.service.createBook(request.toNewBookParameter());
...
}
这里我们引入了一个 NewBookParameter 类,把它当作 service 层创建作品的入口,而 在 resource 中,我们将 NewBookRequest 这个请求类的对象转换成了 NewBookParameter 对象,然后传到 service 层。
在这个结构中,NewBookParameter 属于 service 层,而 NewBookRequest 属于 resource 层,二者相互独立,我们之前纠结的问题也就不复存在了。
但我的问题是,它们两个为什么要一样呢?有了两层不同的参数,我们就可以 给不同层次上的模型以不同的约定了。 比如,对于 resource 层的请求对象,因为它的主要作用是传输,所以,一般来说,我们约 定请求对象的字段主要是基本类型。而 service 的参数对象,因为它已经是核心业务的一 部分,就需要全部转化为业务对象。举个例子,比如,同样表示价格,在请求对象中,我 们可以是一个 double 类型,而在业务参数对象中,它应该是 Price 类型。 我们再来解决 resource 层参数和 service 层参数不一致的情况,现在二者分开了,那我们 就很清楚地知道,其实,就是在业务参数对象构造的时候,传入必需的参数即可。比如, 如果我们需要传入 userId,可以这么做:
class NewBookRequest {
public NewBookParameters toNewBookRequest(long userId) {
...
}
}
@PostMapping("/books")
public NewBookResponse createBook(final NewBookRequest request, final Authenti
long userId = getUserIdentity(authentication);
boolean result = this.service.createBook(request.toNewBookParameter(userId))
...
}
所有的请求对象都属于 resource 层,但在这段代码里, service 层出现了 resource 层的对象,它背离了我们对依赖关系设计的约定,所以,这个 问题就浮出了水面。
案例
代码
@Entity
@Table(name = "user")
@JsonIgnoreProperties(ignoreUnknown = true)
class User {
...
}
这是一个 User 类的声明,它有 @Entity 这个 Anntation,表示它是一个业务实体的对 象,但它的上面还出现了 @JsonIgnoreProperties,这是就是处理 JSON 的一个 Annotation。JSON 会在哪用到,通常都是在传输中。业务实体和传输对象应该具备的特 质在同一个类中出现,显然,这也是没有构建好防腐层的结果,把两个职责混在了一起。
业务代码里的具体实现
@Task
public void sendBook() {
try {
this.service.sendBook();
} catch (Throwable t) {
this.feishuSender.send(new SendFailure(t)));
throw t;
}
}
这里的重点在于, 一旦发送过程出了问题,要通过即时通信工具发送给相关人等,以防系统出现问题无人发 觉。只不过,这里给出的是它最初的样子,也就是通过飞书进行消息发送。
高层模块不应依赖于低层模块,二者应依赖于抽象。
High-level modules should not depend on low-level modules. Both should depend on abstractions.
抽象不应依赖于细节,细节应依赖于抽象。
Abstractions should not depend on details. Details (concrete implementations) should depend on abstractions.
业务代码中任何与业务无关的东西都是潜在的坏味道。
改进
既然我们已经知道了,这些具体的东西是一种坏味道,那该怎么解决呢?你可以引入一个 模型,也就是这个具体实现所要扮演的角色,通过它,将业务和具体的实现隔离开来。
interface FailureSender {
void send(SendFailure failure);
}
class FeishuFailureSenderS implements FailureSender {
...
}
依赖关系是软件开发中非常重要的一个东西,然而,很多程序员在写代码的时候,由于开 发习惯的原因,常常会忽略掉依赖关系这件事本身。现在已经有一些工具,可以保证我们 在写代码的时候,不会出现严重破坏依赖关系的情况,比如,像前面那种 service 层调用 resource 层的代码。 在 Java 世界里,我们就可以用 ArchUnit 来保证这一切。看名字就不难发现,它是把这 种架构层面的检查做成了单元测试,下面就是这样的一个单元测试:
@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);
}
在这里,我们定义了两个层,分别是 Resource 层和 Service 层,而且我们要求 Resource 层的代码不能被其它层访问,而 Service 层的代码只能由 Resource 层方法访问。这就是 我们的架构规则,一旦代码里有违反这个架构规则的代码,这个测试就会失败,问题也就 会暴露出来。
总结
坏味道:依赖混乱
坏味道呈现形态
- 缺少防腐层,业务与外部接口耦合。
- 业务代码中出现具体类。
应对策略
- 引入防腐层,将业务与内部接口隔离。
- 引入模型,将业务与具体实现隔离。
编程规则
高层模块不应依赖于底层模块,二者应依赖于抽象。
抽象不应依赖于细节,细节应依赖于抽象。
记住一句话
代码应该向着稳定的方向依赖。
不一致的代码:为什么你的代码总被吐槽难懂?
大部分程序员对于一致性本身的重要性是有认知的。但通常来说,大家理解的一致性都表 现在比较大的方面,比如,数据库访问是叫 DAO 还是叫 Mapper,抑或是 Repository, 在一个团队内,这是有统一标准的,但编码的层面上,要求往往就不是那么细致了。所 以,我们才会看到在代码细节上呈现出了各种不一致。我们还是从一段具体的代码来分析 问题。
命名中的不一致
enum DistributionChannel {
WEBSITE
KINDLE_ONLY
AL
}
这段代码使用标记作品的分发渠道,从这段代码的内容上,我们可以看到,目前的分发渠 道包括网站(WEBSITE)、只在 Kindle(KINDLE_ONLY),还是全渠道(ALL)。
原因就是,在这里 WEBSITE 和 KINDLE_ONLY 两个名字的不一致。
按照我对一致性的理解,表示类似含义的代码应该有一致的名字,比如,很多团队里都会 把业务写到服务层,各种服务的命名也通常都是 XXXService,像 BookService、 ChapterService 等等。而一旦出现了不一致的名字,通常都表示不同的含义,比如,对于 那些非业务入口的业务组件,它们的名字就会不一样,会更符合其具体业务行为,像 BookSender ,它表示将作品发送到翻译引擎。
显然,这段代码的作者给这两个枚举值命名时,只是分别考虑了它应该起什么名字,却忽 略了这个枚举值在整体中扮演的角色。 理解这一点,改动是很容易,后来,代码被统一成了一个形式:
enum DistributionChannel {
WEBSITE
KINDLE
AL
}
方案中的不一致
public String nowTimestamp() {
DateFormat format = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
Date now = new Date();
return format.format(now);
}
这是一段生成时间戳的代码,当一个系统向另外一个系统发送请求时,需要带一个时间戳 过去,这里就是把这个时间戳按照一定格式转成了字符串类型,主要就是传输用,便于另 外的系统进行识别,也方便在开发过程中进行调试。
从 Java 8 开始,Java 官方的 SDK 借鉴了各种程序库,引入了全新的日期时间解决方案。这 套解决方案与原有的解决方案是完全独立的,也就是说,使用这套全新的解决方案完全可 以应对我们的所有工作。 我们现在的这个项目是一个全新的项目,我们使用的版本是 Java 11,这就意味着我们完全 可以使用这套从 Java 8 引入的日期时间解决方案。所以,我们在项目里的约定就是所有的 日期时间类型就是使用这套新的解决方案。
改进
public String nowTimestamp() {
LocalDateTime now = LocalDateTime.now()
return now.format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss"));
}
为什么一个项目中会出现多个解决方案呢?一个原因就是时间。随着时间流逝,人们会意 识到原有解决方案存在的各种问题,于是,有人就会提出新的解决方案,像我们这里提到 的 Java 日期时间的解决方案,就是 JDK 本身随时间演化造成的。
代码中的不一致
public void createBook(final List<BookId> bookIds) throws IOException {
List<Book> books = bookService.getApprovedBook(bookIds)
CreateBookParameter parameter = toCreateBookParameter(books)
HttpPost post = createBookHttpRequest(parameter)
httpClient.execute(post)
}
这是一段在翻译引擎中创建作品的代码。首先,根据要处理的作品 ID 获取其中已经审核通过的作品,然后,发送一个 HTTP 请求在翻译引擎中创建出这个作品。
通过了解这段代码的背景,你可能已经看出一些端倪了。首先是获取审核通过的作品,这 是一个业务动作,接下来的三行其实是在做一件事,也就是发送创建作品的请求。具体到 代码上,这三行代码分别是创建请求的参数,根据参数创建请求,最后,再把请求发送出 去。这三行代码合起来完成了一个发送创建作品请求这么一件事,而这件事才是一个完整 的业务动作。
public void createBook(final List<BookId> bookIds) throws IOException {
List<Book> books = bookService.getApprovedBook(bookIds)
createRemoteBook(books)
}
private void createRemoteBook(List<Book> books) throws IOException {
CreateBookParameter parameter = toCreateBookParameter(books)
HttpPost post = createBookHttpRequest(parameter)
httpClient.execute(post)
}
一旦我们将不同的关注点分解出来,我们还可以进一步调整代码的结构。像前面拆分出来 的这个方法,我们已经知道它的作用是发出一个请求去创建作品,本质上并不属于这个业 务类的一部分。所以,我们还可以通过引入一个新的模型,将这个部分调整出去:
public void createBook(final List<BookId> bookIds) throws IOException {
List<Book> books = this.bookService.getApprovedBook(bookIds);
this.translationEngine.createBook(books);
}
class TranslationEngine {
public void createBook(List<Book> books) throws IOException {
CreateBookParameter parameter = toCreateBookParameter(books)
HttpPost post = createBookHttpRequest(parameter)
httpClient.execute(post)
..
}
}
从本质上说,我们在做的依然是模型的分层,只不过,这次的出发点是函数的语句。这也 是我一直强调的“分离关注点,越小越好”的意义所在。观察代码的粒度足够小,很多问 题自然就会暴露出来。
总结
坏味道:不一致的代码
坏味道呈现形态
- 命名中的不一致。
- 方案中的不一致。
- 代码中的不一致。
应对策略
- 团队统一编码方案。
- 提取函数,将不同层次的内容放入不同的函数中。
记住一句话
保持代码在各个层面上的一致性。
落后的代码风格:使用“新”的语言特性和程序库升级你的代码
Optional
String name = book.getAuthor().getName();
即便如此,严格地说,这段代码依然是有问题的。因为它没有考虑对象可能为 null 的场 景。 所以,这段代码更严谨的写法是这样:
Author author = book.getAuthor();
String name = (author == null) ? null : author.getName();
然而,在很多真实的项目中,这种严格的写法却是稀有的,所以,在实际的运行过程中, 我们总会惊喜地发现各种空指针异常。
对于这个如此常见的问题,Java 8 中已经给出了一个解决方案,它就是 Optional。 Optional 提供了一个对象容器,你需要从中“取出(get)”你所需要的对象,但在取出 之前,你需要判断一下这个对象容器中是否真的存在一个对象。
class Book {
public Optional<Author> getAuthor() {
return Optioanl.ofNullable(this.author);
}
...
}
Optional<Author> author = book.getAuthor();
String name = author.isPresent() ? author.get() : null;
这种做法和之前做法的最大差别在于,你不会忘掉判断对象是否存在的过程,因为你需要 从 Optional 这个对象容器中取出存在里面的对象。正是这多出来的一步,减少了“忘 了”的概率。 也是因为多了 Optional 这个类,这段代码其实还有更简洁的写法:
Optional<Author> author = book.getAuthor();
String name = author.orElse(null);
有了 Optional,我们可以在项目中做一个约定,所有可能为 null 的返回值,都要返回 Optional,以此减少犯错的几率。
函数式编程
Optional 是 Java 8 引入的新特性,它的出现改变了编写 Java 代码的习惯用法。接下来, 我们来看看另外一个改变我们代码习惯用法的特性。
public ChapterParameters toParameters(final List<Chapter> chapters) {
List<ChapterParameter> parameters = new ArrayList<>();
for (Chapter chapter : chapters) {
if (chapter.isApproved()) {
parameters.add(toChapterParameter(chapter));
}
}
return new ChapterParameters(parameters);
}
不是我们不需要遍历 集合,而是我们有了更好的遍历集合的方式。
public ChapterParameters toParameters(final List<Chapter> chapters) {
List<ChapterParameter> parameters = chapters.stream()
.filter(Chapter::isApproved)
.map(this::toChapterParameter)
.collect(Collectors.toList());
return new ChapterParameters(parameters);
}
在各种程序设计语言中,lambda 都是为了写短小代码提供的便利,所以,lambda 中写出 大片的代码,根本就是违反 lambda 设计初衷的。最好的 lambda 应该只有一行代码。
那如果一个转换过程中有很多操作怎么办呢?很简单,提取出一个函数,就像前面代码中 的 toChapterParameter,它负责完成从 Chapter 到 ChapterParameter 的转换。这样一 来,列表转换的本身就完全变成了一个声明,这样的写法才是能发挥出列表转换价值的写 法。
总结
坏味道:落后的代码风格
具体案例
- Java 8 引入的Optional。
- 函数式编程。
核心要点
- 引入Optional 可以减少由于程序员的忽略而引发空对象的问题。
- 懂得最基本的几个操作:map、filter和reduce,就可以把大部分集合操作转换成列表转换。
编程规则
- 声明式编程。
- 写短小的函数,不要在lambda中写过多的代码。
记住一句话
不断学习”新”的代码风格,不断改善自己的代码。
多久进行一次代码评审最合适?
有一个发现坏味道的实践,就是代码评审,也就是很多人熟悉的 Code Review, Wikipedia 上定义是这样的:
代码评审,是指对计算机源代码系统化地审查,常用软件同行评审的方式进行,其目的 是在找出及修正在软件开发初期未发现的错误,提升软件质量及开发者的技术。
代码评审是一个沟通反馈的过程
看待代码评审还有一个团队视角,代码评审的 过程,也是一个知识分享的过程,保证一些细节的知识不再是隐藏在某一个人的头脑中, 而是放置到了团队的层面。
把这样的理解放到代码评审中,就是要尽可能多暴露问题,尽可能多做代码评审。
暴露问题
做代码评审时,我们可以从下面几个角度来看代码:
- 实现方案的正确性;
- 算法的正确性;
- 代码的坏味道。
正常情况一切顺利,异常情况却考虑不足。
及时评审
我对评审的建议是,提升评审的频率,比如,每天评审一次。
评审周期过长是有问题的,周期过长,累积的问题就会增多,造成的结果就是太多问题让 人产生无力感。如果遇到实现方案存在问题,要改动的代码就太多了,甚至会影响到项目 的发布。 而提升评审的频率,评审的周期就会缩短,每个周期内写出来的代码就是有限的,人是有心力去修改的。
总结
代码评审
代码评审的本质是沟通反馈
- 尽可能多地暴露问题。
- 尽可能多地做代码评审。
代码评审的角度
- 实现方案正确性。
- 算法正确性。
- 代码坏味道。
极限编程
代码评审推演到极致是结对编程。
记住一句话
代码评审暴露的问题越多越好,频率越高越好。
新需求破坏了代码,怎么办?
一次驳回的实现
我们的系统里有这样一个功能,内容作品提交之后要由相应的编辑进行审核。既然有审 核,自然就有审核通过和不通过的情况,这是系统中早早开发完成的功能。 有一天,新的需求来了:驳回审核通过的章节,让作品的作者重新修改。造成作品需要驳 回的原因有很多,比如,审核标准的调整,这就会导致原先通过审核的作品又变得不合格 了。 在实现这个需求之前,我们先来看看代码库里已经有怎样的基础。 首先,系统里已经有了审核通过和审核不通过的接口。
PUT /chapter/{chapterId}/review
DELETE /chapter/{chapterId}/review
在这个设计里,将章节(chapter)的审核(review)当作了一个资源。在创建章节的时 候,章节的审核状态就创建好了。审核通过,就相当于对这次审核进行了修改,而审核不 通过,就相当于删除了这个资源。 对应着这两个接口,就有两个对应的服务接口:
class ChapterService {
public void approve(final ChapterId chapterId) {
...
}
public void reject(final ChapterId chapterId) {
...
}
}
顾名思义,approve 函数对应着审核通过,而 reject 对应着审核不通过。相应地,章节上 有一个状态字段,标识现在章节处于什么样的状态。章节是待审核、审核通过,还是审核 不通过,就是通过这个字段标记出来的。
class Chapter {
private Status status = Status.PENDING;
public void approve() {
this.status = Status.APPROVED;
}
public void reject() {
this.status = Status.REJECTED;
}
}
当 我们想对外提供一个接口时,我们必须问一下,真的要提供一个新接口吗?
在原有的业务中,审核通过会进入到下一个阶段,而审核不通过,就会退回到作者那里进 行修改。那驳回之后呢?它也会要求作者去修改。 说到这里,你就不难发现了,驳回的动作和审核不通过,二者的后续动作是一样的。它们 的差别只是起始的状态,如果原来的状态是待审核,经过一个审核不通过的动作,状态就 变成了审核不通过;而如果原来的状态是审核通过,经过一个驳回的动作,状态就变成了 驳回。所以,我们完全可以复用原来的审核不通过接口。 既然是复用接口,所有的变化就全部都是内部变化了,我们可以根据章节当前的状态进行 判断,设置相应的状态。具体到代码上,我们既不需要增加驳回的接口,也不需要增加驳 回的服务,只需要在 Chapter 类内部进行修改,代码改动量比原先预期的就小了很多。
class Chapter {
private Status status = Status.PENDING;
...
public void reject() {
if (status == Status.PENDING) {
this.status = Status.REJECTED;
return;
}
if (status == Status.APPROVED) {
...
.
}
}
}
按照这个理解,我们只要增加一个驳回的状态,在当前状态是审核通过时,将这个新状态 赋值上去就可以了。
看上去,我们已经把这次要改动的代码限制在一个最小的范围。但其实,我还想再问一个 问题,我们真的需要这么一个状态吗? 是否增加一个驳回的状态,回答这个问题还是要回到业务上,驳回后续的处理与审核不通 过的状态有什么不同。 按照产品经理本来的需求,他是希望做出一些不同来,比如,处于审核不通过的状态,编 辑端是无法查看的,而处于驳回状态的,编辑是可以查看的。但在当前的产品状态下,我 们是否可以将二者统一起来呢?也就是说,都按照审核不通过来处理呢?
一次定时提交的实现
在我们的系统中,一般情况下,作者写完一章之后就直接提交了,这是系统中已经实现好 的一个功能。现在来了新的需求,有时候,作者会囤一些稿子,为了保证自己每天都有作 品提交,作者希望作品能够按自己设定的时间去提交,也就是说,一个章节在它创建的时 候,并不会直接提交到编辑那里去审核,而是要到特定的时间之后,再来完成作品的提 交。 实际上,“每天都有作品提交”就是一种连续的签到,通常来说,系统都会给连续签到以 奖励,这也是对于作者的一种激励手段。 如果你面对这样一个需求,你会怎么实现呢?
class Chapter {
// 章节 ID
private ChapterId chapterId;
// 章节标题
private String title;
// 章节内容
private String content;
// 章节状态
private Status status;
// 章节创建时间
private ZonedDateTime createdAt;
// 章节创建者
private String createdBy;
// 章节修改者
private String modifiedBy;
// 章节修改时间
private ZonedDateTime modifiedAt;
...
}
我似乎已经看到你跃跃欲试的样子了。你可能会想:这个实现还不简单,在章节上加上一 个调度时间就行了:
class Chapter {
...
private ZonedDateTime scheduleTime;
}
有需求就改动实体,这几乎是很多人不假思索的编码习惯,然而,对于一个业务系统而 言,实体是其中最核心的部分,对它的改动必须有谨慎的思考。
随意修改实体,必然伴随着其它部分的调整,而经常变动的实体,就会让整个系统难以稳 定下来。一般来说,一个系统的业务并不会经常改变,所以,核心的业务实体应该是一个 系统中最稳定的部分。
不放在 Chapter 这个类里面,那要放到哪呢?很显然,这里少了一个模型,一个关于调度 的模型。我们只要增加一个新的模型,让它和 Chapter 关联在一起就好了:
class ChapterSchedule {
private ChapterId chapterId;
private ZonedDateTime scheduleTime;
...
}
分析到这里,我们突然发现,模型里居然没有一个地方可以存放提交时间,是的,我们需 要修改实体了,我们要给它增加一个提交时间:
class Chapter {
...
private ZonedDateTime submittedAt;
}
一个字段该不该加在一个类上,取决于其改变的原因。前面的定时时间确 实不该加,而这里的提交时间却是应该加的。提交时间本来就是章节的一个属性,只不过 如前面所说,之前,这个信息与创建时间是共用的,而如今,因为定时提交的出现,二者 应该分开了。
总结
新需求破坏的代码
坏味道出现的高发地带
- 增加新接口。
- 改动实体。
编程的原则
- 对外提供的接口越少越好。
- 仔细分析实体扮演的角色。
记住一句话
谨慎地对待接口和实体的变动。
熊节:什么代码应该被重构?
“低内聚”会直接引发的现象是“霰弹式修改。
每当需要对某个事情做出修改时,你都必须在许多不同的类内做出许多小修改,那么就 可以确定,你所面临的坏味道是霰弹式修改。当需要修改的代码分散在多处,你不但很 难找到它们,也很容易忘记某个重要的修改。
而“高耦合”直接引发的现象则是有某种相似性、但又表现不同的“发散式变化 (Divergent Change)”:
如果某个类经常因为不同的原因在不同的方向上发生变化,发散式变化就出现了。
对于“霰弹式修改”,解决的办法是使用“搬移函数”和“搬移字段”,把所有需要修 改的代码放进同一个模块;
对于“发散式变化”,解决的办法是首先用“提炼函数”将不同用途的逻辑分开,然后 用“搬移函数”将它们分别搬移到合适的模块;
对于“过长的消息链”,你应该使用“隐藏委托关系”;对于“中间人”,对症的疗法 则是“移除中间人”,甚至直接“内联函数”。
总结
从本质上说,static 函数是一种全局函数,static 变量是一种全局变量,全局的函数和变 量是我们尽量规避的; 一个函数调用了 static 函数不好测试; 除了写程序库,日常开发尽可能不用 static 函数。
那怎么消除 static 函数呢?消除 static 函数,最简单的做法就是用普通的函数调用替换掉 static 函数,也就是把这里的 static 都去掉。涉及到相应的字段,也要去掉 static。这种做 法没有问题,但通常这种做法太粗暴了。这里我们尝试着用重构的方式一步一步地把它替 换掉。
直观的坏味道
- 使用缩写。
- 用I表示接口,用Impl表示实现。
- 使用static函数或变量。
- 使用singleton模式。
- 写得潦草的README。
需要注意
重构不是大开大合的过程,而是一系列细碎而微小的操作
记住一句话
尽量不适用static。