This commit is contained in:
louzefeng
2024-07-09 18:38:56 +00:00
parent 8bafaef34d
commit bf99793fd0
6071 changed files with 1017944 additions and 0 deletions

View File

@@ -0,0 +1,111 @@
<audio id="audio" title="14 | 多久进行一次代码评审最合适?" controls="" preload="none"><source id="mp3" src="https://static001.geekbang.org/resource/audio/e6/4b/e6c5d978df3d6932e2811dd89e47264b.mp3"></audio>
你好,我是郑晔。
前面我们讲了很多代码的坏味道,我们的关注点都在代码本身上。知道了什么样的代码是坏味道,有了具体的评判标准。那么,该如何去运用坏味道这把“尺子”呢?
有一个发现坏味道的实践,就是代码评审,也就是很多人熟悉的 Code ReviewWikipedia上定义是这样的
>
代码评审,是指对计算机源代码系统化地审查,常用软件同行评审的方式进行,其目的是在找出及修正在软件开发初期未发现的错误,提升软件质量及开发者的技术。
大多数程序员都经历过代码评审,也都能够初步理解代码评审本身存在的价值,这也是差不多全行业都认为有价值的一个实践。只不过,每个团队在代码评审的实践差别还挺大的,有的团队是在一个完整的开发周期结束之后,做一次代码评审;有的是安排每周的代码评审;有的则是每天都要做代码评审。之所以会有这样的差异,主要就是团队对于代码评审本身的理解有差异。
所以,这一讲我们就来谈谈,到底应该如何理解代码评审。
## 代码评审是一个沟通反馈的过程
关于代码评审,第一个问题就是,为什么要做代码评审?
这个问题其实比较简单,没有人能够保证自己写出来的代码是没有问题的,而规避个体问题的主要方式就是使用集体智慧,也就是团队的力量。
这个答案是从个体的角度在看问题,其实,看待代码评审还有一个团队视角,代码评审的过程,也是一个知识分享的过程,保证一些细节的知识不再是隐藏在某一个人的头脑中,而是放置到了团队的层面。
不过,无论是从哪个角度看代码评审,它的本质,就是**沟通反馈**的过程。我把我对这段代码的理解分享给你,你把你对这段代码的想法共享给我。有人给出代码实现的知识,有人贡献出对技术的理解。
如果我们理解了代码评审是一个沟通反馈的过程,那就可以把沟通反馈的一些原则运用到代码评审中。
我在《[10x 程序员工作法](https://time.geekbang.org/column/intro/100022301)》里,花了一个模块的篇幅讲了沟通反馈,我们希望沟通要尽可能透明,尽可能及时。把这样的理解放到代码评审中,就是要**尽可能多暴露问题,尽可能多做代码评审。**
## 暴露问题
我们先来说暴露问题。代码评审就是一个发现问题的过程,这是一个大家都能理解的事情。但问题就在于,要发现什么问题?
如果泛泛地回答,那自然就是代码实现中的各种问题。然而,这个答案还可以细化一下,做代码评审时,我们可以从下面几个角度来看代码:
- 实现方案的正确性;
- 算法的正确性;
- 代码的坏味道。
我们一个一个来看,先来说实现方案。理论上说,实现方案应该是设计评审中关注的内容,但在实际工作中,并不是所有团队都能够很好地执行设计评审,而且设计评审有时也关注不到特别细的点,所以,一些实现方案的问题只有在代码评审中才能发现。
在一次代码评审中,我看到一个批量处理的 REST 接口接到请求经过一些处理之后它会调用另外一个服务因为这个服务只支持单一的请求所以REST 接口只能一个一个地向这个服务发送请求。
如果一切正常的话,这个接口是没有问题的。但是,如果在处理过程中出现失败,没有把所有的请求发给另一个服务,这个接口的行为是什么样呢?是需要客户端重新发起请求,还是服务端本身重新调用接口?如果是服务端负责重试,那么,这个方案本身没有任何重试的机制,也就是说,一个请求一旦出错,它就丢了,业务不能顺利地完成。
当我把这个问题抛了出来时,同事一下子愣住了。显然,他只考虑了正常的情况,而没有考虑出现失败的情况。把它做成一个完整的方案,很可能还需要做一个后台服务,负责替未能得到有效处理的任务善后,显然,这就不是代码调整,而是整个方案的调整。
这是很多程序员,尤其是经验比较少的程序员写程序经常会出现的问题:**正常情况一切顺利,异常情况却考虑不足。**
我们再来说说算法正确性。
别看整个行业都十分重视算法,但那是在面试的过程中。真正到了实际工作里,算法复杂度常常被人忽略。
我们之前讲过嵌套的代码,对于循环语句,我们要把处理一个元素的代码提取出来。不过,这有时候也会带来一些意想不到的问题。
有一次代码评审,我看到了一段写得很干净的代码,就是把循环里对于一个元素的处理拆了出去。还没等我来赞美这段代码写得好,我就看到了单个元素处理的代码,每次都要查询一次数据库,找出相应的元素,做修改之后再存回去。
就这样单独看每段代码都是对的但合在一起就出了问题本来可以通过一次查询解决的问题变成了N次查询。
我再给你讲一个让我印象深刻的故事。在我职业生涯的初期,我做过一段时间图像识别的工作。有一次,一个实习生说自己的代码太慢了,让我帮忙看看。
从表面上看,代码写得还不错,不是一眼能够看出问题。仔细看了半天,我在一个遍历图像像素点的循环里发现了一个图像复制的代码,也就是说,每循环一次,都要把整个图像复制一遍,代码慢就在所难免了。
我相信,如果这是一个算法练习,这两个同事都能够有效地解决这个问题,但放在工程里,就难免挂一漏万了。所以,算法正确性也是我们要在代码评审中关注的。
无论是实现方案的正确性,还是算法的正确性,对于大多数团队来说,都会关注到。但代码坏味道却是很多团队容易忽略的,这里面的关键点就是很多团队对于坏味道的标准太低了。
在这个专栏里,我讲了很多坏味道,有一些是你早就认同的,有一些则在挑战你的认知。也正是因为有这些挑战你认知的部分,所以很多代码即便经过评审,也依然会产生很多问题。关于坏味道,我们整个专栏都在说,更多的细节我就不在这里讨论了。
## 及时评审
说完代码评审中要暴露的问题,我们再来说说代码评审的另外一个方面,代码评审的频率。
不同的团队代码评审,频率是不一样的,最糟糕的肯定是不评审,整个团队闭着眼睛向前冲,这就不是我们关心的范畴。常见的评审频率是每个迭代评审一次,也有每周评审的。
我对评审的建议是,提升评审的频率,比如,每天评审一次。
评审周期过长是有问题的,周期过长,累积的问题就会增多,造成的结果就是太多问题让人产生无力感。如果遇到实现方案存在问题,要改动的代码就太多了,甚至会影响到项目的发布。
而提升评审的频率,评审的周期就会缩短,每个周期内写出来的代码就是有限的,人是有心力去修改的。学过我任何一个专栏的同学都知道,我在专栏中反复强调短小的价值,只有及时的沟通反馈,才有可能实现这一原则。
你或许会好奇,我们是不是可以再进一步提升评审的频率呢?
肯定可以,如果把代码评审推至极致,就是有个人随时随地来做代码评审。我在《[10x程序员工作法](https://time.geekbang.org/column/intro/100022301)》讲过极限编程的理念,就是把好的实现推向极致,而代码评审的极致实践就是结对编程。
结对编程就是两个人一起写一段代码,一个人主要负责写,一个人则站在用外部视角保证这段代码的正确性。好的结对编程对两个人的精力集中度要求是很高的,两个人一起写一天代码其实是很累的一件事,不过,也正是因为代码是两个人一起写,代码质量会提高很多。
从我之前经历的一些团队实践来看,结对编程还有一个额外的好处,就是对于团队中的新人提升极大,这就是拜结对编程这种高强度的训练和反馈所赐。高强度的训练和反馈,本质上就是一种刻意练习,而刻意练习是一个人提升最有效的方式。
我知道,对于大多数团队来说,是没有条件做大规模的结对编程的。但对个体来说,创造一些机会与高手一起写代码也是很好的。即便不能一起写,去观摩高手写代码也能学到很多东西。再退一步,实在身边没有机会,去网上看看高手写代码也是一种学习方式。
## 总结时刻
今天的加餐我们讨论了代码评审。对于很多人来说,代码评审只是一个发现问题的过程,而通过今天的讨论,我们知道了代码评审是一个沟通反馈的过程。站在沟通反馈的角度,我们关注的是,尽可能多地暴露问题,尽可能多地做代码评审。
代码评审可以从实现方案正确性、算法正确性和代码坏味道的角度去发现问题。代码评审的频率是越高越好,频率越高,发现和解决问题的难度越低,团队越容易坚持下去。
如果把代码评审推向极致就是随时随地做代码评审,这个实践就是结对编程。
如果今天的内容你只能记住一件事,那请记住:**代码评审暴露的问题越多越好,频率越高越好**。
<img src="https://static001.geekbang.org/resource/image/af/f8/afc1e5ae5yyf843680880108efce7af8.jpg" alt="">
## 思考题
你在代码评审上有哪些经验,或者遇到过哪些让你印象深刻的问题代码,欢迎在留言区分享你的经验。如果你有所收获,也欢迎把这节课分享出去。
感谢阅读,我们下一讲再见!
参考资料:[27 | 尽早暴露问题: 为什么被指责的总是你?](https://time.geekbang.org/column/article/84374)

View File

@@ -0,0 +1,242 @@
<audio id="audio" title="15 | 新需求破坏了代码,怎么办?" controls="" preload="none"><source id="mp3" src="https://static001.geekbang.org/resource/audio/ae/f3/aec0b5d011e21f6b074c0ea08d56f6f3.mp3"></audio>
你好,我是郑晔。
我前面课程讲的所有坏味道都是告诉你如何在已有的代码中发现问题。不过你要明白,即便我们能够极尽所能把代码写整洁,规避各种坏味道,但我们小心翼翼维护的代码,还是可能因为新的需求到来,不经意间就会破坏。
一个有生命力的代码不会保持静止,新的需求总会到来,所以,**写代码时需要时时刻刻保持嗅觉。**
这一讲加餐,我来给你讲讲两个发生在真实项目中的故事。
## 一次驳回的实现
我们的系统里有这样一个功能,内容作品提交之后要由相应的编辑进行审核。既然有审核,自然就有审核通过和不通过的情况,这是系统中早早开发完成的功能。
有一天,新的需求来了:驳回审核通过的章节,让作品的作者重新修改。造成作品需要驳回的原因有很多,比如,审核标准的调整,这就会导致原先通过审核的作品又变得不合格了。
在实现这个需求之前,我们先来看看代码库里已经有怎样的基础。
首先,系统里已经有了审核通过和审核不通过的接口。
```
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 这个类里面,那要放到哪呢?很显然,这里少了一个模型,一个关于调度的模型。我们只要增加一个新的模型,让它和 Chapter 关联在一起就好了:
```
class ChapterSchedule {
private ChapterId chapterId;
private ZonedDateTime scheduleTime;
...
}
```
有了这个模型,后续再有关于调度的信息就可以放到这个模型里面了,而更重要的是,我们的核心模型 Chapter 在这个过程中是保持不变的。
我们之所以要把定时提交的信息与章节本身分开,因为这二者改变的原因是不同的。你或许已经发现了,是的,如果将二者混在一起,就是违反了单一职责原则。对于一个程序员来说,深入理解单一职责原则是非常必要的。
到这里,定时提交的问题看上去已经得到了一个很合理的解决,有了基础的数据结构,修改对应的接口和服务,对大多数程序员来说,都是一件驾轻就熟的事情。那么,这个讨论就结束了吗?我们可能暂时还不能停下来。
我们新增的需求是定时发布,之所以要有这么个需求,因为这和作者的激励是相关的。要想确定作者的激励,就要确定章节的提交时间,问题是,我们怎么确定章节的提交时间呢?
在原来实现中,创建时间就是提交时间,因为章节是立即提交的,而现在创建时间和提交时间有可能不同了。
你可能会想到,创建时间不行,那就用修改时间。我告诉你,这也不行,修改时间是章节信息最后一次修改的时间,它有可能因为各种原因变更,最简单的就是编辑审核通过,这个时间就会变。
分析到这里,我们突然发现,模型里居然没有一个地方可以存放提交时间,是的,我们需要修改实体了,我们要给它增加一个提交时间:
```
class Chapter {
...
private ZonedDateTime submittedAt;
}
```
到这里,估计有些人已经懵了。前面我们辛辛苦苦地讨论,为的就是不在 Chapter 里增加信息,而这里,我们竟然就增加了一个字段。
前面我们说了,一个字段该不该加在一个类上,取决于其改变的原因。前面的定时时间确实不该加,而这里的提交时间却是应该加的。提交时间本来就是章节的一个属性,只不过如前面所说,之前,这个信息与创建时间是共用的,而如今,因为定时提交的出现,二者应该分开了。
或许你还有一个疑问,我们难道不能直接用 submittedAt 去存储调度时间吗?严格地说,不行。因为调度时间可能与具体提交的时间有差异。我举个例子,因为某种原因,系统宕机了,启动之后,调度任务执行,这时可能已经过了调度时间很多了,但这个时候提交章节,它的时间就不会是调度时间。
至此,我们完整地分析完了定时提交的实现,你还记得我们为什么要做这个分析吗?没错,因为它要改动核心的实体,而这又是一个坏味道的高发地带。
## 总结时刻
这一讲,我用了两个例子给你讲了新需求到来时需要关注的地方,它们分别是:
- 增加新接口;
- 改动实体。
接口和实体,其实也是一个系统对外界产生影响的重要部分,一个是对客户端提供能力,一个是产生持久化信息。所以,我们必须谨慎地思考它们的变动,它们也是坏味道产生的高发地带。
对于接口,我们对外提供得越少越好,而对于实体,我们必须仔细分析它们扮演的角色。
如果今天的内容你只能记住一件事,那请记住:**谨慎地对待接口和实体的变动**。
<img src="https://static001.geekbang.org/resource/image/7d/75/7d256ee63083871919363af3b2281e75.jpg" alt="">
## 思考题
你平时是怎么对待接口和实体的变动的呢?欢迎在留言区分享你的经验。
感谢阅读,我们下一讲再见!
参考资料:
[34 | 你的代码是怎么变混乱的?](https://time.geekbang.org/column/article/87845)
[20 | 单一职责原则:你的模块到底为谁负责?](https://time.geekbang.org/column/article/258222)

View File

@@ -0,0 +1,117 @@
<audio id="audio" title="16 | 熊节:什么代码应该被重构?" controls="" preload="none"><source id="mp3" src="https://static001.geekbang.org/resource/audio/d7/60/d7fdd9f2yyc7e18a45ac08f51a818f60.mp3"></audio>
你好,我是郑晔。
代码坏味道的说法源自《[重构](https://book.douban.com/subject/30468597/)》这本书,坏味道和重构这两个概念几乎是如影随形。提及《重构》这本书,在国内谁还能比《重构》两版的译者熊节更了解它呢?所以,这一讲,我就请来了我的老朋友熊节,谈谈在他眼中看到的重构和坏味道。有请熊节老师!
你好,我是熊节。
自从翻译了《重构》以后,很多公司找我去做重构的培训,光是华为一家,这个主题在各个不同的部门就培训过好些次。每次讲这个主题,我都觉得挺为难的:重构这事有什么可培训的呢,不就是一个无脑模式匹配的事吗!然而跟各家公司的读者们一交流,我就发现事情并没有那么简单。
很多人一说到重构,就聊到虚无缥缈的事上了,像什么架构啦、文化啦,等等。我不得不先把他们拉住仔细问问,他们是怎么读《重构》这本书的?这一问我就发现,原来很多读者(恐怕是绝大多数读者),还没弄明白这本书到底应该怎么读。
## 什么代码应该被重构?
《重构》这本书,以及重构这门手艺,提纲挈领的部分,都在一个关键的问题上:**什么代码应该被重构**。
你可能会说,质量不好的代码需要被重构。没错,可是代码的质量到底应该如何评判呢?
首先我们要明确的是,代码的好与坏不应当用个人好恶、“含混的代码美学”来表达,因为这会带来两个困难:
- 第一,每个人对于“好”或“美”的观念可能相当不同;
- 第二,对于坏代码缺乏明确的“症状”判断,也就很难提出明确的改进措施。
即便是一些经典的程序设计原则,也有同样的问题。例如“高内聚低耦合”,尽管这是所有人都赞同的设计原则,但究竟什么样的代码呈现了“低内聚”、什么样的代码呈现了“高耦合” 、“低内聚”与“高耦合”是否总是同时出现、应该以何种办法提高内聚降低耦合……这些问题仍然是悬而未决的。
因此,对于真正在一线工作的人来说,“高内聚低耦合”很多时候就成了一句咒语,念完咒语后,呼唤出的其实还是每个人原本的编程习惯与风格,并不真正指导任何行为的改变。
而当我们去观察“低内聚高耦合”带来的问题时事情就变得明朗了。比如当我们仔细阅读《重构》第三章时我们会发现“低内聚”会直接引发的现象是“霰弹式修改Shotgun Surgery
>
每当需要对某个事情做出修改时,你都必须在许多不同的类内做出许多小修改,那么就可以确定,你所面临的坏味道是霰弹式修改。当需要修改的代码分散在多处,你不但很难找到它们,也很容易忘记某个重要的修改。
而“高耦合”直接引发的现象则是有某种相似性、但又表现不同的“发散式变化Divergent Change
>
如果某个类经常因为不同的原因在不同的方向上发生变化,发散式变化就出现了。
我再举另一个设计原则的例子。“迪米特原则”也是常被提及的面向对象设计原则之一然而知道这个名称是一回事知道如何识别不符合迪米特原则的代码则又需要更多的个人经验。《重构》第三章则把这个原则表述为两个非常直观的症状“过长的消息链Message Chains”和“中间人Middle Man”。
>
如果你看到用户向一个对象请求另一个对象,而后者再次请求另一个对象,然后再请求另一个对象……这就是消息链。在实际代码中,你看到的可能是一长串取值函数,或者一长串临时变量。
>
人们可能过度运用委托。你也许会看到某个类接口有一半的函数都委托给其他类,这样就是过度运用。
你发现没对于开始我们提到的“什么代码应该被重构”这个关键问题虽然《重构》作者Martin Fowler和Kent Beck非常客气地声称“并不试图给你一个何时必须重构的精确衡量标准”实际上《重构》给出的24项“坏味道”在《重构》第一版中是22项已经形成了一个非常明确的代码质量检查清单。
尽管这本书从未声称这是一份完备的坏味道清单,但在实际工作中,还不用说完全识别并消除这份列表中的全部坏味道,只要能做到命名合理、没有重复、各个代码单元(类、函数等)体量适当、各个代码单元有明确且单一的职责、各个代码单元之间有恰当的交互,这就已经是质量相当高的代码了。
更重要的是,伴随着对具体症状的了解,对症的解决办法也变得明确。在《重构》第三章里非常明确地讲到:
- 对于“霰弹式修改”,解决的办法是使用“搬移函数”和“搬移字段”,把所有需要修改的代码放进同一个模块;
- 对于“发散式变化”,解决的办法是首先用“提炼函数”将不同用途的逻辑分开,然后用“搬移函数”将它们分别搬移到合适的模块;
- 对于“过长的消息链”,你应该使用“隐藏委托关系”;对于“中间人”,对症的疗法则是“移除中间人”,甚至直接“内联函数”。
这就是我前面所说的“无脑模式匹配”。
讲到这里,你也就明白了,对于绝大多数程序员而言,阅读和使用《重构》这本书的正确方法就应该是:
1. 打开任意一段代码(可以是自己刚写完的或者马上要动手修改的);
<li>翻开《重构》第三章,遍历其中的每个坏味道:
<ol>
<li>识别这段代码中是否存在上述坏味道;
<ol>
1. 如有,则遵循该坏味道所列的重构手法,对该段代码进行重构;
1. 如无,则继续遍历代码。
上述过程不需要玄妙的理论和含混的代码美学,只需要机械的重复和简单的模式匹配。正因为此,重构才是一项完完全全具备可操作性、能够在任何遗留代码库上实践的技术。
## 培养对“坏味道”的判断力
当然,每位实践者仍然“**必须培养出自己的判断力,学会判断一个类内有多少实例变量算是太大、一个函数内有多少行代码才算太长”。**
就在最近我看到某大厂的一位“代码委员会理事”在文章里说某段代码“挺好的长度没超过80行逻辑比较清晰”。而在我看来一个函数超过7行就已经是“太长”这还是在考虑到Java语法比较啰嗦的前提下。这就是不同实践者“自己的判断力”所体现的差异。
尽管从来没有明确指定对每个函数或类的代码行数要求,但“对象健身操”这篇文章(见于《[ThoughtWorks文集](https://www.infoq.cn/minibook/thoughtworks-anthology)》提出的9项规则已经有非常明确的指向
- 方法中只允许使用一级缩进;
- 不允许使用else关键字
- 封装所有的原生类型和字符串;
- ……
在这样的规则约束下写出一个超过10行的函数将是相当困难的实际上在“规则6保持实体对象简单清晰”中已经明确提出每个类的长度不能超过50行
正如“对象健身操”这篇文章的作者Jeff Bay自己所说这套“健身操”的意义在于“在一个简单的项目里尝试一些比以前严格得多的编码标准……会迫使你更为严格地以面向对象的风格编写代码”从而“以一种全新的方式思考你的代码”。
不过这得需要你刻意练习。正所谓“台上一分钟,台下十年功”,**缺乏在受控环境下的刻意练习,很难通过工作中的自然积累提升判断力。**
另外对正确的代码构造足够熟悉也是很重要的一个基本功这个观点最早是Kent Beck的《[实现模式](http://book.douban.com/subject/3324516/)》这本书中提到的。什么意思呢?
传说旧时民间古董店的学徒需要先在仓库里看真货,看得多了,见到假货时就会本能地提起警觉。对于代码也是一样:程序员需要熟悉正确的代码构造,在看到有问题的代码构造时才会本能地提起警觉。并且,**“正确的代码构造”并非无穷无尽,实际上在单线程编程中,几十个常见的模式已经几乎能够完全覆盖所有场景。**
Kent Beck在前言里说“这是一本关于如何编写别人能懂的代码的书”尽管他还谦虚地说这本书“不是模式书籍”但实际上《实现模式》充分地展现了“模式”的本意它提供了一整套“用代码表述意图”的模式语言这套语言能让程序员在最短的时间内学会如何写出具有表现力的代码并且自然而然地远离坏味道。
**从一开始就以合理的方式编程********从而使坏味道不要出现,我想这才是负责任的程序员应该采取的工作方式。**
当然,极限编程的各种实践,尤其是工程技术实践彼此紧密相关。例如自动化测试、持续集成、集体代码所有制的缺失,都会导致代码的坏味道更容易堆积。而从另一个角度来看,这些实践从任何一个切入,又都会自然地引导出其他相关的实践。
**一位“知行合一”的程序员最终会发现,极限编程是唯一合理且有效的软件开发方法**。最终,只有采用以可工作的软件为核心的软件开发方法,才能得到高质量的可工作的软件,这就是《敏捷宣言》第二句关于坏味道的终极答案。
## 郑老师说
好了,熊节老师的分享就到这里,我是郑晔。
熊节老师对于问题的分析总是这么一针见血。重构就是一个模式匹配的过程,识别出坏味道,运用对应的重构手法解决问题。坏味道是一切重构的起点,而识别坏味道不是靠个人审美,而要依赖通用的标准。
我的这个专栏就是把一些坏味道用更直接的代码形式展现在你面前,让你可以日常的工作中,不断地锻炼自己的代码嗅觉。
## 思考题
经过熊节老师的讲解,你是不是对重构和坏味道有了新的认识呢?欢迎在留言区分享你更新过的想法。
感谢阅读,我们下一讲再见。

View File

@@ -0,0 +1,256 @@
<audio id="audio" title="17 | 课前作业点评:发现“你”代码里的坏味道" controls="" preload="none"><source id="mp3" src="https://static001.geekbang.org/resource/audio/9f/6c/9fb4629884da39a25bb0fe418cfa986c.mp3"></audio>
你好,我是郑晔。
在这个专栏刚开始的时候,我给你留了一个课前作业,实现一个待办事项管理的软件。许多同学都利用自己的业余时间完成了这个作业,感谢大家的付出!
学习代码的坏味道,听别人讲是一种方式,但这种方式总会让人有一种隔岸观火的感觉,虽然知道有问题,但感觉并不深刻。最直接受益的方式就是自己写了代码,然后,让别人来点评。其实,这就是某种形式的代码评审。
所以,这一讲,我们就来做一次“代码评审”,直接来看看代码中存在的问题。题目背景我就不再做过多的介绍了,如果没有来得及完成作业的同学,可以先到“[课前作业区](https://time.geekbang.org/column/article/325594)”回顾一下题目。
既然是指出问题,得罪大家可能就在所难免了,希望你不要介意,毕竟能够发现自己的问题是精进的第一步。好,我们开始!
## 从已知的坏味道出发
在[极客双同学的代码仓库](https://github.com/benben773/todomaster)里,我在[一段代码](https://github.com/benben773/todomaster/blob/main/todomaster/src/main/java/com/test/service/impl/ProcessTxtServiceImpl.java)中看到了之前我们课程中讲过的坏味道:
```
Item itemNew = new Item(item.getName());
itemNew.setUserIndex(userIndex);
itemNew.setIndex(initUserIndex);
```
我们的业务需求是添加TODO项这段代码就是在这个过程中创建一个新的TODO项对象。那这段代码有什么问题一方面这里有 setter另一方面这里的 setter 只在初始化的过程中用到。显然,我们可以用一个更完整的构造函数替换掉它。
其实,从这段代码出发,我们还能看到一些小问题,比如,这里创建 TODO 项设置了两个字段,一个是 userIndex一个是 index。index 可以理解,表示这个 TODO 项的索引,但 userIndex 是什么呢?你需要仔细阅读代码才能发现,它其实是一个用户的标识,表示这个索引项是由某个用户创建的。既然是用户标识,按照通常的做法它可以叫 userId这就降低了理解的难度。
这段代码所在类的声明也是一个让人出戏的地方:
```
public class ProcessTxtServiceImpl implements ProcessItemservice
```
这个类实现了一个接口 **ProcessItemservice**,显然,这里的拼写是有问题的,它应该是 ProcessItemService另外它的名字叫做“处理TODO项的服务”一方面在一个服务名字上用了处理这个动词另一方面“处理”这个名字也是特别泛化的一个名字。如果是我来声明这个接口它可能就叫 **ItemService**
所以,你可以看到,仅仅是一个接口的命名,就有这么多的问题。
我们再来看这个类的命名 **ProcessTxtServiceImpl**,这个名字里有一个 Txt 是容易让人困惑的,一般来说,如果不是特别的原因,**尽量不要用缩写**。
我初看到这个名字时着实想了半天它表示什么含义一开始我以为是表示事务Transaction常有人把事务缩写成 Tx如果它的含义是表示事务那么这里就是一个拼写错误了。后来我才想明白这里的 Txt 表示的是文本Text**仅仅省了一个字母,却造成了理解上更大的障碍,实在有些得不偿失。**
如果 Txt 表示的是文本,这里就暴露出另外一个问题。这里为什么要有一个文本呢?其实是对应着另外一个数据库存储的实现,这是第四阶段的要求。
文本和数据库的差别到底是体现在哪里呢?体现在存储上。而在这段代码中,差别从服务层面就开始了,换言之,按照这段代码的逻辑,实现数据库存储,就需要把整个的业务逻辑重新写一遍。显然,这种做法是从结构上来看是有问题的,会造成大量的重复代码。
理解了文本和数据库只差别在存储这件事,我们再回过头来看这个类的声明。
```
public class ProcessTxtServiceImpl implements ProcessItemservice
```
这个为数据库预留的实现根本就是不需要的,只有一个 ItemService 的实现就够了,换言之,也就没有必要声明出一个接口,这里的类层次这么复杂,根本就是没有必要的。
```
public class ItemService
```
这里我再补充一个点,很多 Java 程序员给类命名有个不好的习惯用“I” 打头给接口命名用“Impl”给实现类结尾这其实是早期的一种编程习惯准确地说这就是没有想好命名的偷懒方式。其实它也是我们讲到的“[用技术术语命名](https://time.geekbang.org/column/article/326166)”的一种具体体现方式。后来的代码基本上就不这么做了,因为我们可以找到更准确的描述。但很多人的编程习惯却留在了早期,所以,这也算是一种遗毒的吧。
## 一个“静态”的问题
接下来,我们再来看一个很多人代码中都存在的问题。
下面是来自[刘大明](https://github.com/liudaming/todo)同学的[一段代码](https://github.com/liudaming/todo/blob/main/src/main/java/com/timegeekbang/todo/user/UserContext.java),这是一个用以存放用户信息的类。单看这段代码本身,其实写得还是非常不错的,代码本身并不长,而且考虑了很多的细节。我们暂且忽略其它的细节,我注意到这段代码的主要原因是因为它用到了 static
```
public class UserContext {
private static ThreadLocal&lt;Integer&gt; USERID = new ThreadLocal();
private UserContext() {
throw new UnsupportedOperationException();
}
public static String getUserID() {
return String.valueOf(USERID.get());
}
public static void setUserID(Integer userID) {
USERID.set(userID);
}
}
```
在《10x 程序员工作法》讲到[测试驱动开发](https://time.geekbang.org/column/article/78104)时,我曾经讲了 static 函数的问题,简单总结一下就是:
- 从本质上说static 函数是一种全局函数static 变量是一种全局变量,全局的函数和变量是我们尽量规避的;
- 一个函数调用了 static 函数不好测试;
- 除了写程序库,日常开发尽可能不用 static 函数。
那怎么消除 static 函数呢?消除 static 函数,最简单的做法就是用普通的函数调用替换掉 static 函数,也就是把这里的 static 都去掉。涉及到相应的字段,也要去掉 static。这种做法没有问题但通常这种做法太粗暴了。这里我们尝试着用重构的方式一步一步地把它替换掉。
首先,我要去掉这里的构造函数,因为这里的构造函数是私有的,无法调用,而我们要用普通的函数,自然就需要构造出一个对象来。
```
public class UserContext {
private static ThreadLocal&lt;Integer&gt; USERID = new ThreadLocal();
public static String getUserID() {
return String.valueOf(USERID.get());
}
public static void setUserID(Integer userID) {
USERID.set(userID);
}
}
```
然后,我们需要找到对应的调用点,这里就以其中的一个为例,下面就是在退出登录的地方调用了这里的 static 函数:
```
public class UserAccounts {
...
public void loginOut() {
UserContext.setUserID(null);
}
}
```
我们可以把它改成对象的调用:
```
public class UserAccounts {
...
public void loginOut() {
new UserContext().setUserID(null);
}
}
```
这样,我们就有了一个对象,因为原来的函数是 static 函数,所以,这里的调用,本质上还是原来的函数,所以不会有影响。
然后,我们把这个创建出的对象变成这个类的字段,如果你使用的是支持重构功能的 IDE这就是一个快捷键的操作引入字段Introduce Field
```
public class UserAccounts {
...
private UserContext context = new UserContext();
public void loginOut() {
context.setUserID(null);
}
}
```
如果在一个类有多个调用点,不妨都改成这个新字段的函数调用,正如我们前面所说,目前还是一个 static 函数,无论从哪个对象调用,调用的都是同一个函数。
通常来说,这个 static 函数应该不只是在一个类中使用,所以,它应该是在多个类中间共享的,为了保证多个类中间使用同一个 UserContext 对象UserContext 对象的初始化就不能在这个类进行,而要在同一个地方初始化,所以,我们这里可以把 UserContext 对象作为构造函数的参数传进来:
```
public class UserAccounts {
...
private UserContext context;
public UserAccounts(..., final UserContext context) {
...
this.context = context;
}
public void loginOut() {
context.setUserID(null);
}
}
```
有了这个基础,我们再在 UserAccounts 这个对象初始化的时候,把这个 UserContext 对象传进来:
```
new UserAccounts(..., new UserContext());
```
如此一来UserContext 这个对象的初始化就放到对象组装的过程中了,这就可以在多个不同的对象组件中共享这个对象了。如此往复,将所有的调用点都这么修改,我们就消除了对于 static 函数的依赖。现在,我们可以动手消除 static 了:
```
public class UserContext {
private ThreadLocal&lt;Integer&gt; USERID = new ThreadLocal();
public String getUserID() {
return String.valueOf(USERID.get());
}
public void setUserID(Integer userID) {
USERID.set(userID);
}
}
```
消除 static 函数本身并不难,这里我是借着这个简单的例子,给你演示一下,如何一步一步地进行重构。可能这比很多人以为的大刀阔斧地修改代码来得要琐碎得多,**但只有这样一点一点调整,代码足够安全,每一步都是能够停下来的。**
无论如何,请别忘了,真正能给予我们修改有效性回答的是,**单元测试**。
估计很多人看到这里就会说,如果 static 都成了坏味道,那 Singleton 模式该怎么办呢?答案就是**尽可能不用 Singleton 模式**。我在《软件设计之美》中讲[可测试性](https://time.geekbang.org/column/article/241094)和[设计模式](https://time.geekbang.org/column/article/265121)时,都说到过 Singleton 模式,简单地说,系统里只有一个实例和限制系统里只能构建出一个实例,这是两件事,而且,如果一个函数牵扯到 Singleton 类也不好测试。
在一些同学的代码中,我也看到的 Singleton 模式的使用,处理手法其实与这里消除 static 函数是类似的只不过Singleton 稍微好一点的是,它的函数和字段本身都已经是普通的类成员了,我们只需要把那个限制实例唯一的 static 函数和字段消除就可以了。
说了半天的代码问题,我还想对很多人普遍忽略的小问题说上几句,这就是文档,对应到各位的代码库中,主要就是 README。
一个开源项目的好坏与否,同它的文档质量是强相关的。我知道,作为程序员,大家的普遍兴趣都是写代码,所以,文档就常常被忽略了。
如果我不了解这个项目的背景,很多人的 README 给我提供的信息量是非常有限的。
大家的 README 普遍存在的问题有两种,一种是信息量太少,比如,只写了如何构建一个项目,另一种是把 README 当成 blog在里面写了自己的心得体会。无论是哪种信息的有效性都很差。
README 文件是一个项目的门面它应该给我们提供关于这个项目的背景信息比如这个项目是做什么的、当前的状态、如何入手等等。你可以找一些经典的开源项目去看看好的README是怎么写的。**好的程序员要学会表达,不仅仅会用代码表达,也要会用文字表达**。
好,这就是大家作业中的所有问题了吗?当然不是,代码中存在的问题还很多。不过,你不用担心,即便这个专栏的正式更新结束了,我也会考虑以加餐的形式,继续我们这个云端的代码评审环节。所以,之前没来得及写代码的同学依然可以继续写,说不定下次就会谈到你的代码。
## 总结时刻
今天我们点评了大家代码中存在的一些问题,除了之前在专栏中讲到过的坏味道,今天我们还讲到了一些一眼就可以看出问题的坏味道:
- 使用缩写;
- 用 I 表示接口,用 Impl 表示实现;
- 使用 static 函数或变量;
- 使用 singleton 模式;
- 写得潦草的 README。
你在写代码时也要注意这些问题。
我还借着 static 函数的调整过程,给你演示了如何一步一步地重构代码,保证代码的安全。希望你能够理解,重构不是大开大合的过程,而就是这样细小而微的操作。
如果今天的内容你只能记住一件事,那请记住:**尽量不使用 static**。
另外按照最初的约定我也选出了3位作业完成比较好的同学分别是[邓志国](https://github.com/bobdeng/todolist)、[LiuTianyou](https://github.com/LiuTianyou/todo)、[_CountingStars](https://github.com/mgxian/todolist) ,这几天极客时间的团队会联系你们邮寄奖品。
<img src="https://static001.geekbang.org/resource/image/80/30/802cfa950b97bc944e0619afef945830.jpg" alt="">
## 思考题
我们今天谈到了文档,你平时写文档吗?或者,你平时阅读项目文档,发现什么值得改善的地方吗?欢迎在留言区分享你的经验。
参考资料:[加餐 | 你真的了解重构吗?](https://time.geekbang.org/column/article/85915)