关于代码审查的一些探讨和总结

青旅半醒 2023-02-27 10:44 61阅读 0赞

> 对于以往开发过的项目，在事后一直在进行一些深刻的反思。第一个大型项目的开发过程中，出现了灾难性的bug事件，因项目积攒bug太多，甚至在项目工期安排上，专门安排近半个月时间进行bug方面的修改和维护。
> 
> 因为公司属于初创公司，缺乏相应的规范制度，并且开发人员普遍年轻，大多工作经验不足3年，甚至还有很多1年工作经验的开发人员。这也是导致项目最后出现灾难bug的原因之一。
> 
> 个人刚开始也是开发者，也曾经开发过很多bug，再之后转职过产品，项目助理，大数据开发，再到现在的运维工程师，多个职位的转变，给了更多的学习机会和接触新知识的机会。个人对项目管理，敏捷开发，Code Review，公司发展等方面内容十分感兴趣，故经常对这方面进行阅读和思考。
> 
> 经阅读众多Code Review血的经历和教训之后，发现一个好的，优质的项目，无论在任何条件下，也不应该抛弃代码审查。我公司项目经常因工期紧，没时间等各种原因推辞了代码审查，导致了每个项目几乎都存在着严重的bug，不规范代码事件。当项目积攒到一定程度之后，如果出现同事离职， 或需求变更等方面内容，发现其他开发人员很难对原开发人员代码进行二次开发或修改，只能被迫重新梳理需求，重新开发，严重影响了工作效率，也出现了大量大量的冗余代码。经专业代码质量评审工具扫描，经常能发现很多重复代码，着实影响了项目的质量。
> 
> 我对Code Review的思考，总结为三个字母，就是WWH。W（why）W（who）H（How）—即，为什么要做代码审查？谁来做代码审查？如何进行代码审查？
> 
> 针对个人思考的这三方面，对Code Review进行一些探讨和总结。

# 一、、why？为什么要做代码审查？ #

## 1.1 不审查的坏处 ##

代码的质量反应了产品质量，产品不稳定、老是出现BUG，直接影响客户体验。

同时，代码的好坏决定了未来运维的成本，如果因为一时疏忽和妥协，回头又没有及时修改，中间又出现人员变动，那么这份代码的后患是无穷的。

因为不规范，可读性差，对交接人来说从心态上是本能反抗的，但是又不得不改，于是就一通乱改，能贴膏药就贴膏药，能运行就可以，管他规范不规范。这样导致的后果是，代码从不规范走向更加不规范，很难想象经过5-10年持之以恒的不规范，这个产品最终是否还能如愿以偿的存活下去。

技术债务的危害怎么形容都不为过，轻则系统局部异常，中等的会导致修改困难，严重的需要推翻重来。

从物理学上看，熵让我们理解了一件事，如果不施加外力影响，事物永远向着更混乱的状态发展，所以规范和审查就显得弥足珍贵了。

从软件设计看，软件设计要关注长期变化，需要应对需求规模的膨胀。如果腐烂的代码日积月累，这些在不断流变腐烂的东西又怎能支撑起长期的变化呢！

## 1.2 评审的好处 ##

代码审查是一个低投入、高产出的开发活动，就个人而言，从其中学到的习惯、方法和知识，获益匪浅。

代码评审的作用很多，主要表现在 6 个方面。

*  好处 1，\*\*尽早发现 Bug 和设计中存在的问题。\*\*问题发现得越晚，修复的代价越大。代码审查把问题的发现尽量提前，自然会提高效能。
 *  好处 2，\*\*提高个人工程能力。\*\*不言而喻，别人对你的代码提建议，自然能提高你的工程能力。事实上，仅仅因为知道自己的代码会被同事审查，你就会注意提高代码质量。
 *  好处 3，\*\*团队知识共享。\*\*一段代码入库之后，就从个人的代码变成了团队的代码。代码审查可以帮助其他开发者了解这些代码的设计思想、实现方式等。另外，代码审查中的讨论记录还可以作为参考文档，帮助他人理解代码、查找问题。
 *  好处 4，**针对某个特定方面提高质量**。一些比较专业的领域，比如安全、性能、UI 等，可以邀请专家进行专项审查。另外，一些核心代码或者高风险代码，也可以通过团队集体审查的方式来保证质量。
 *  好处 5，\*\*统一编码风格。\*\*这，也是代码审查的一个常见功能，但最好能通过工具来实现自动化检查。
 *  好处 6，\*\*社会性功用。\*\*如果你在编程，而且知道一定会有同事将检查你的代码，那么你编程的姿势和心态就会完全不同。这之间的微妙差异正是在于会不会有人将对你的代码做出反馈与评价。

## 1.3 Code Review能有什么好处？ ##

*  **首先是团队知识共享的角度**

一个开发团队中，水平有高有低，每个人侧重的领域也有不同。怎么让高水平的帮助新人成长？怎么让大家都对自己侧重领域之外的知识保持了解？怎么能有人离职后其他人能快速接手？这些都是团队管理者关心的问题。

而代码审查，就是一个很好的知识共享的方式。通过代码审查，高手可以直接指出新手代码中的问题，新手可以马上从高手的反馈中学习到好的实践，得到更快的成长；通过代码审查，前端也可以去学习后端的代码，做功能模块A的可以去了解功能模块B的。

可能有些高手觉得给新手代码审查浪费时间，自己也没收获。其实不然，新人成长了，就可以更多的帮高手分担繁重的任务；代码审查中花时间，就少一些帮新人填坑擦屁股的时间；良好的沟通能力、发现问题的能力、帮助其他人成长，都是技术转管理或技术上更上一层楼必不可少的能力，而通过代码审查可以有效的去练习这些方面的能力。

*  **然后是代码质量的角度**

现实中的项目总是人手缺进度紧，所以被压缩的往往就是自动化测试和代码审查，结果影响代码质量，欠下技术债务，最后还是要加倍偿还。

也有人寄希望于开发后的人工测试，然而对于代码质量来说，很多问题通过测试是测试不出来的，只能通过代码审查。比如说代码的可读性可维护性，比如代码的结构，比如一些特定条件才触发的死循环、逻辑算法错误，还有一些安全上的漏洞也更容易通过代码审查发现和预防。

也有人觉得自己水平高就不需要代码审查了。对于高手来说，让别人审查自己的代码，可以让其他人学习到好的实践；在让其他人审查的同时，在给别人说明自己代码的时候，也等于自己对自己的代码进行了一次审查。这其实就跟我们上学时做数学题一样，真正能拿高分的往往是那些做完后还会认真检查的。

*  **还有团队规范的角度**

每个团队都有自己的代码规范，有自己的基于架构设计的开发规范，然而时间一长，就会发现代码中出现很多不遵守代码规范的情况，有很多绕过架构设计的代码。比如难以理解和不规范的命名，比如三层架构里面UI层绕过业务逻辑层直接调用数据访问层代码。

如果这些违反规范的代码被纠正的晚了，后面再要修改就成本很高了，而且团队的规范也会慢慢的形同虚设。

通过代码审查，就可以及时的去发现和纠正这些问题，保证团队规范的执行。

## 1.4 代码质量评价标准 ##

较常用的评价标准，其中包括：编码规范、可读性、可维护性、重复度及可测试性。

**编码规范**  
主要包含是否遵守了最佳实践和团队编码规范，是否包含可能出问题的代码，以及可能存在安全的漏洞。编码规范有助于提高团队内协助的效率以及代码的可维护性。

**可读性**  
Code Review 是一个很好的测验代码可读性的手段。如果你的同事可以轻松地读懂你写的代码，那说明你的代码可读性很好；反之则说明你的代码可读性有待提高了。遵守编码规范也能让我们写出可读性更好的代码。

**可维护性**  
代码的可维护性是由很多因素协同作用的结果。代码的可读性好、简洁、可扩展性好，就会使得代码易维护；更细化地讲，如果代码分层清晰、模块化好、高内聚低耦合、遵从基于接口而非实现编程的设计原则等等，那就可能意味着代码易维护。除此之外，代码的易维护性还跟项目代码量的多少、业务的复杂程度、利用到的技术的复杂程度、文档是否全面等诸多因素有关。

**重复度**  
遵守 Don’t Repeat Yourself 原则，尽量减少重复代码的编写，复用已有的代码。对项目定期进行代码重复度检测是一个很有意义的事，可以帮助开发人员发现冗余代码，进行代码抽象和重构。重复的代码一旦出错，意味着加倍的工作量和持续的不可控。如果代码中有大量的重复代码，就要考虑将重复的代码提取出来，封装成公共的方法或者组件。

**可测试性**  
代码可测试性的好坏，同样可以反应代码质量的好坏。代码的可测试性差，比较难写单元测试，那基本上就能说明代码设计得有问题。

除此之外还有很多代码质量评价标准。我们需要一些取舍，选取部分大家有共识的规则定义团队好的代码标准。

# 二、who?谁来做代码审查？ #

1.  首要参与人员，必须是**项目经理**。项目经理管理了所有的项目开发任务，由项目经理来组织协调进行代码审查，也能使项目经理清楚的明白，目前团队的开发水平。
2.  若多个业务之间由不同的人进行开发，负责这个业务流转链路的下一个环节的同事，需参加代码评审活动。
3.  新入职的工作人员，代码风格千千万万，只有规范统一的代码风格才能整体提高公司开发效率和产品质量。
4.  技术总监类的人物，技术能力较高，代码能力较强的人参与代码评审，提出指导性的建议和规范，经商量协定后制定一份公司内部规范文档，这份文档是受到开发人员广泛认可的，这样才能得到良好的执行。

# 三、how?如何进行代码审查？ #

## 3.1 把Code Review变成一种开发文化而不仅仅是一种制度 ##

把Code Review 作为开发流程的必选项后，不代表Code Review这件事就可以执行的很好，因为Code Review 的执行，很大部分程度上依赖于审查者的认真审查，以及被审查者的积极配合，两者缺一不可！

如果仅仅只是当作一个流程制度，那么就可能会流于形式。最终结果就是看起来有Code Review，但没有人认真审查，随便看下就通过了，或者发现问题也不愿意修改。

真要把Code Review这件事做好，必须让Code Review变成团队的一种文化，开发人员从心底接受这件事，并认真执行这件事。

要形成这样的文化，不那么容易，也没有想象的那么难，比如这些方面可以参考：

*  首先，得让开发人员认识到Code Review这件事为自己、为团队带来的好处
 *  然后，得要有几个人做好表率作用，榜样的力量很重要
 *  还有，对于管理者来说，你激励什么，往往就会得到什么
 *  最后，像写自动化测试一样，**把Code Review要作为开发任务的一部分，给审查者和被审查者都留出专门的时间去做这件事**，不能光想着马儿跑得快又舍不得给马儿吃草

如何形成这样的文化，有心的话，还有很多方法可以尝试。只有真正让大家都认同和践行，才可能去做好Code Review这件事。

## 3.2 代码评审需要关注点有哪些？ ##

代码评审应该关注的重点：

*  是否明显的逻辑错误
 *  是否落实了代码规范
 *  代码的可读性和可维护性是否良好
 *  代码是否有违背基本的设计模式理念

代码评审的目的：最大化维护团队的利益。

为了软件的良好发展和团队的高效协作，每个人的代码最好看上去都差不多，这样修改别人的代码就比较亲切。

## 3.3 代码评审的流程 ##

1.  约定规范文档----只有先制定好规范，才可以遵照规范执行，有法可依。
2.  制定评审频率----多久进行一次代码评审，应该有一个制定的时间，是按照项目来评审，还是按照时间。
3.  完善代码规范----知识沉淀，形成闭环。对已经发现的问题，及时处理。提高开发人员的整体代码水平，完善相应的代码规范文档。

对代码的审查则是作为开发流程的一个必选项，每次开发新功能或者修复Bug，开一个新的分支，分支要合并到master有两个必要条件：

*  所有的自动化测试通过
 *  有至少一个人Code Review通过，如果是新手的PR，还必须有资深程序员Code Review通过

## 3.4 一些代码评审的方案探讨 ##

有code diff 和code review。

*  code diff 一般在大量重构或者比较复杂或者特殊逻辑实现的时候做。一般是写法什么的讲给组内其他人。
 *  code review是在基于branch开始提交pr之后等有人review之后才merge。

这时候的review第一是让别人检查自己的代码，第二告诉团队自己了那些改动，一旦review过来拿代码就属于团队的，有什么问题不再是写代码的人的事情。

任何提交的代码或者说明都需要被评审一下，并且评审全部内容。

合并到主干之前进行代码评审。核心业务逻辑代码或者核心业务板块的代码需要被评审，评审主要揪出来重复代码、代码的命名和行为代码不规范。

在两个场景下做Code Review：

*  个人代码提交前做Code Review。Review自己将要Commit的代码。
 *  团队每日Code Review。不同的项目按照团队约定时间进行，可以在一天的开始或者将要结束的时候进行。

那相对应的被评审的代码：

*  个人所有需要提交的代码需要在Commit之前Review
 *  团队Code Review。需要Review所有被提交的代码。有时候团队内会存在前端和后端。可以选择一起做Code Review，也可以选择分开。

以上两种情况，需要关注代码和Commit信息。代码中需要关注业务代码，测试代码，以及当天的测试核心代码覆盖率等信息。

评审内容就是

*  可以按照提交记录逐条进行Code Review，看到代码的演进；也可以review多次提交的结果集。
 *  Code Review时，需要关注Commit记录和代码，Commit记录需要直白的描述当次提交的目的。
 *  针对问题，表明观点并达成团队共识。

遇到问题我们就：

*  首先划定个讨论的范围，确定讨论将会聚焦。描述清楚是什么问题，为什么有这个问题，有哪些解决办法。
 *  避免只是挑错。Code Review 时团队内学习的过程，同时也是开发团队思路达成共识的过程，不会针对某个人，主要针对问题。
 *  做好Code Review 记录。主要需要记录时间、问题的位置、问题的修改方案。根据团队自身情况，可以扩展记录的内容。记录的形式可以使用Confluence，也可以选择直接在问题处添加Todo。
 *  Code Review时除了发现问题，还会遇到写的好的代码，也需要及时的赞美，同时传播一些编程技巧和工具。

## 3.5 Code Review 的时机 ##

在以往的工作经验中，**Code Review 越是靠左移，修改代码的成本越低，开发人员的修改意愿也就越高**，那什么叫左移？

我们看一下软件开发的流水线和个人认为最合理的 code review 时机：

![image-20210204154554376][]

软件工程的开发流水线（图）

从流水线上来说，有些人会在临近上线，**在靠右的地方**合并 master 的时候才进行 code review，这个时候修改成本就很高，因为代码已经测试过，如果因为 code review 有问题需要重新修改代码，那么**功能本身又要回归测试，占用的测试双倍的时间，对于人力资源是双倍的浪费**，因为已经临近上线，却因为 code review 被打回，开发人员愿意重构代码的意愿也会很低，如果明明发现问题，又因为上线压力，不打回不符合规范的代码，那么久而久之大家失去对 code review 的敬畏心理，code review 也会慢慢变成形式化，应用发布流程而已，既不能提高代码质量，降低系统 Bug，也不能提升开发人员的水平，反而降低的开发团队的效率，所以**选择在上线前进行 code review 不是一个好主意**，所以从性价比上来说 code review 最好的时机应该是在 **功能分支自测完成后，需要合并到 develop 分支申请提测前** 通知项目组成员对增量的代码进行 code review。

所以，代码审查要高效的话，核心就是要追求**快速反馈**，越早发现代码问题修改的成本就越低，具体参考下图：

![极客时间-研发效率破局之道（图）][-]

这里需要注意的是，代码在经过机器扫描后（**这里有一个技巧就是可以在 GitLab CI 加入自动的代码风格检查，代码静态扫描是一个高频操作，一天可能会有几十，甚至上百次的 Commit，如果接入 GitLab CI 实现自动化静态扫描，大家不需要在自己本地执行静态扫描，那么效率也会大大的提升**），项目组成员只需要把注意力放在 **代码逻辑结构，功能设计的可维护，可扩展性 等机器不容易发现问题的地方上**，然后就完成代码审查。因为代码还未提测，所以就算 Merge Request 不合格被打回后，因为还未提测，也不会占用测试人员的资源，开发人员的修改意愿也会更高，总体来说是可以达到高效和质量的要求。

# 四、代码评审技巧 #

## 4.1 评审的关键操作 ##

*  **第一步**，规定提交说明一定要包括标题、描述和测试情况三部分，但暂时还不具体要求必须写多少字。比如，测试部分可以简单写一句“没有做测试”，但一定要写。如果格式不符合要求，审查者就直接打回。这个格式要求工作量很小，比较容易做到，两个星期后整个团队就习惯了。虽然只是在提交说明里增加了简单描述，也已经为审查者和后续工作中进行问题排查提供一些必要信息，所以大家也比较认可这个操作。
 *  **第二步**，要求提交说明必须详细写明测试情况。如果没有做测试一定要写出具体理由，否则会被直接打回。这样做，不但为审查者提供了方便，还促进了开发人员的自测。整个团队在一个多月后，也养成了详细描述测试情况的习。
 *  **第三步**，逐步要求提交的原子性。我要求每一个提交要在详细描述部分描述具体实现了哪些功能。如果一个提交同时实现了多个功能，那就必须解释为什么不能拆开提交；如果解释不合理的话，提交直接被打回。

提交说明是提高代码审查的利器，好的格式应该包含以下几个方面：

*  **标题**，简明扼要地描述这个提交。这部分最好在 70 个字符之内，以确保在单行显示的时候能够显示完整。比如，在命令行常用的 git log —oneline 输出结果要能显示完全。
 *  **详细描述**，包括提交的目的、选择这个方法的原因，以及实现细节的总结性描述。这三个方面的内容最能帮助审查者阅读代码。
 *  **测试情况**，描述的是你对这个提交做了什么样的测试验证，具体包括正常情况的输出、错误情况的输出，以及性能、安全等方面的专项测试结果。这部分内容，可以增加审查者对提交代码的了解程度以及信心。
 *  **与其他工具和系统相关的信息**，比如相关任务 ID、相关的冲刺（sprint，也可翻译为“迭代”）链接。这些信息对工具的网状互联提供基础信息，非常重要。这里还有一个 Git 的技巧是，你可以使用 Git 的提交说明模板（Commit Message Template），来帮助团队使用统一的格式。

## 4.2 对评论进行分级 ##

在做Code Review时，需要针对审查出有问题的代码行添加评论，如果只是评论，有时候对于被审查者比较难甄别评论所代表的含义，是不是必须要修改。

建议可以对Review的评论进行分级，不同级别的结果可以打上不同的Tag，比如说：

*  【blocker\]: 在评论前面加上一个blocker标记，表示这个代码行的问题必须要修改
 *  \[optional\]：在评论前面加上一个\[optional\]标记，表示这个代码行的问题可改可不改
 *  \[question\]：在评论前面加上一个\[question\]标记，表示对这个代码行不理解，有问题需要问，被审查者需要针对问题进行回复澄清

# 五、代码质量、代码审查、代码评审优秀博文 #

*  [说透代码评审][Link 1]
 *  [Code Review最佳实践][Code Review]
 *  [在淘宝，我们是这样衡量代码质量的][Link 2]

Java 语言常见的风格有以下几种规范：

1.  [Order Java SE 的标准规范：https://www.oracle.com/technetwork/java/codeconvtoc-136057.html][Order Java SE _https_www.oracle.com_technetwork_java_codeconvtoc-136057.html]
2.  [Google Java 开发规范： https://google.github.io/styleguide/javaguide.html][Google Java _ https_google.github.io_styleguide_javaguide.html]
3.  [阿里巴巴 Java 开发手册：https://github.com/alibaba/p3c ][Java _https_github.com_alibaba_p3c]（国内常用）

[image-20210204154554376]: /images/20230209/01126c3a705a4952a1f9bb050fa574f2.png
[-]: https://img-blog.csdnimg.cn/img_convert/c8317f2bc31c6321afc0cf51730f4a85.png
[Link 1]: https://www.cnblogs.com/jackyfei/p/13299877.html
[Code Review]: https://www.cnblogs.com/dotey/p/11216430.html
[Link 2]: https://segmentfault.com/a/1190000037523068
[Order Java SE _https_www.oracle.com_technetwork_java_codeconvtoc-136057.html]: https://www.oracle.com/technetwork/java/codeconvtoc-136057.html
[Google Java _ https_google.github.io_styleguide_javaguide.html]: https://google.github.io/styleguide/javaguide.html
[Java _https_github.com_alibaba_p3c]: https://github.com/alibaba/p3c