代码评审赋魅

先来看一个令无数技术Leader闻风丧胆的项目“死亡”三角，业务压力引起代码质量降低，代码质量降低引起开发效率降低，开发效率降低又加剧了业务压力，最终致使业务压力山大，乃至项目烂尾。如何破解？方法有不少，像精简业务需求、增长开发人手、升级技术架构等，不少时候须要多管齐下，但凡打掉这个“死亡”三角中的任何一角，就能终止这个恶性循环，甚至逆转为良性循环。

代码评审（Code Revew，简称CR）的首要打击目标显然是“烂代码”。避免“烂代码”的最好时机是写代码的时候，其次是代码评审的时候。IBM 的 Orbit 项目有 50 万行代码，使用了 11 级的代码检查（其中包含代码评审），结果是项目提早交付，而且只有一般预期错误的 1%。一份对 AT＆T 的一个 200 多人组织的研究报告显示，在引入代码评审后，生产率提升了 14％，缺陷减小了 90％。那到底什么是代码评审？如何进行代码评审？继续往下看。

1 CR 祛魅

我我的认为代码有这几种级别：1）可编译，2）可运行，3）可测试，4）可读，5）可维护，6）可重用。经过自动化测试的代码只能达到第3）级，而经过CODE REVIEW的代码少会在第4）级甚至更高。

—— 陈皓

下面 8 条有关 CR 的阐述，你以为哪些是正确的？

1. 搞形式主义，存粹是浪费时间
2. CR 是保证程序正确性的手段
3. CR 是保证代码规范的手段
4. CR 是 Leader 的事，跟我不要紧
5. 我只看指给个人 CR，其余 CR 跟我不要紧
6. 没有时间 CR，直接 Merge
7. CR 必须一行不落从头看到尾
8. CR 必须一次完成

———————————————————————————————— 请仔细思考 60 秒

3...2...1...时间到，你的答案是几条？很抱歉，在我看来，没有一条是正确的。一、四、五、6 是送分题，显然都是错误的。7 是眉毛胡子一把抓，CR 就像读书，不是全部的书都适合精度，也不是全部的代码都须要评审。8 是任务心态，为了 CR 而 CR，CR 的目的不是完成 CR，而在于提高代码质量，你写代码时也不会一次完成全部功能。比较有争议的是 2 和 3，诚然，正确性和代码规范都是 CR 要关注的方面，但这并不意味着 CR 要保证正确性和代码规范（CR 也无法保证），保证正确性的主要手段是测试（单元测试，集成测试，契约测试，功能测试，自动化测试等），而保证代码规范主要依靠代码规范检查工具（像经常使用的 checkstyle 和 PMD）。

CR 究竟是什么？依我所见，CR 本质上是一种讨论，一种严肃的、专业的、异步的以文字形式呈现的讨论，随意性和情绪化是 CR 的大忌。什么叫随意性？未经审视的评论。什么叫情绪化？因时而异，因人而异。高水平的 CR 首先要忘掉本身。

2 知：CR 的三重境界

技术水平决定了 CR 的下限，认知高度决定了 CR 的上限。因此说 CR 水平高不高，最终仍是看认知水平。认识 CR 有三重境界，分别是执行层、团队层和文化层。

2.1 执行层：昨夜西风凋碧树，独上高楼，望尽天涯路

第一层为执行层，顾名思义就是经过如何作来认识 CR。如下列举 CR 时需重点关注的六个方面，并辅以相应的例子便于理解。

1）关注代码规范。命名是第一位的，一个使人费解的命名背后每每隐藏着一个设计纰漏。其余诸如空白字符、换行、注释等问题，也会影响代码的可读性和可理解性。

2）避免重复代码。编程法则第一条，Don't repeat yourself. 重复代码是万恶之首，重复代码人人得而诛之！

3）下降圈复杂度。什么是圈复杂度？简单来讲就是代码中 if/case/for/while 出现的次数。圈复杂度越高，BUG 率越高。若是一个方法的圈复杂度达到 3 或者更高，那么 CR 时就要多看两眼。

4）关注性能问题。性能问题虽然不常见，可一旦暴雷每每就是大问题。CR 时看到循环，记得多留一个心眼。

5）关注分布式事务。涉及远程服务调用，或者跨库更新的场景，都应考虑是否存在分布式事务问题，以及适用何种处理方式，是依赖框架保证强一致性，仍是记录异常数据保证最终一致性，抑或是直接忽略？

6）关注架构设计。代码有代码规范，架构有架构规范。面对一个新功能的 MR（Merge Request），除了检查架构规范，还应推敲其架构设计，好比是否符合整洁架构三原则，无依赖环原则，稳定依赖原则，稳定抽象原则。

除了线上 CR，还有一种特殊的线下 CR 方法，就是跳过 MR，直接拉取代码，进行总体 CR，将评审意见在代码中标记为 TODO 或者 FIXME，而后 @ 相关开发进行改进。这样作最大的好处，就是避免受单个 MR 的影响，掉入只见树木不见森林的陷阱。

2.2 团队层：衣带渐宽终不悔，为伊消得人憔悴

接下来再看第二层，如何从团队视角认识 CR。前面说了，CR 本质上是一种讨论，培根说过「读书令人完整，讨论令人完备」，从我的到团队，CR 分别意味着什么？

• 提高自我觉察能力：这是从我的视角来看，当你知道你写的代码会有另外一双眼睛来审阅，那你写代码时就会保持一份警觉，放弃天知、地知、我知、你不知的幻想，认认真真写好每一行代码。
• 创建良好开发节奏：这是从团队视角来看，CR 是团队的同步器，每一个人既是本身 MR 的做者，又是别人 MR 的评审者，从 MR 到 CR，再从 CR 到 MR，构成了每一个工做日最动听的乐章。
• 高频次的团队活动：这也是从团队视角来看，CR 既然是讨论，那么就不只仅是一我的的事，而是一种团队活动，一种高频次、高质量、低成本的极具性价比的团队活动。

2.3 文化层：众里寻他千百度，蓦然回首，那人却在，灯火阑珊处

最后是文化层，CR 既是传帮带文化的重要组成，又是工程师文化的平常体现。

• 传帮带文化的重要组成：资深工程师 CR 初级工程师的代码，能够给予高频次、高质量的指导；初级工程师 CR 资深工程师的代码，能够欣赏、学习高手如何把玩代码，取其精华去其糟粕。
• 工程师文化的平常体现：协做、高效、进取、影响力，这些在各大互联网公司的工程师文化中频频出现的关键词，无一不与 CR 紧密相连。不夸张的说，工程师文化香不香，就看 CR 作的好很差。

3 行：CR 高效之法

认识完 CR，咱们再来探讨一下如何高效的进行 CR。在我看来，高效 CR 首先有赖于如下几个客观条件和主观条件。

客观上来看，和谐的工程师文化和清晰的代码规范是高效 CR 的两块基石。所谓和谐的工程师文化，就是说团队对代码秉持开放的心态，不藏着掖着，以写好代码为荣，以写坏代码为耻，持续关注代码质量。而清晰的代码规范，一方面提升了代码的可读性，另外一方面也统一了编码风格，极大的减小了不一样代码风格对评审者注意力的干扰。

主观上来看，对评审者而言，第一要端正态度，保持谦卑的心态，人非圣贤孰能无过，择其善者而从之，其不善者而改之。第二要谨记评审的对象是代码，而不是人，你写下的每一条评审意见都应基于客观事实和数据，作到有理有据，不带我的情绪。

基于我多年 CR 的实操经验，结合Google Code Review Developer Guide，我整理了一些高效 CR 的最佳实践，供你参考：

• 依据我的偏好天天固定几个时间段专门用于 CR，个人习惯是出门前和下班前。CR 耗费的脑力丝绝不亚于编码，甚至更高，CR 过程当中须要高度集中注意力。清醒的头脑和无干扰的环境，是提出高质量的评审意见的秘诀。
• 除了固定时间段，任务切换期间也是 CR 的好时机。
• 每次 CR 尽可能控制在 15~30 分钟之内，超过 30 分钟应休息一会。
• 收到 MR 以后，先判断一下 MR 的性质，若是是 Bug Fix 类型的 MR，应尽快评审，若是是新功能 MR，则能够等待下一个 CR 窗口。
• 从收到 MR 到 CR 结束，最长不要超过 1 个工做日。
• 开始 CR 以前先要搞清楚 MR 要解决的问题背景。
• CR 就像读书，先看目录（改动的文件列表），再精读重点章节（包含核心业务逻辑的代码），最后扫读剩余章节。
• 若是改动的文件数量较多，能够打开 IDE，切换到源分支，方便在 CR 过程当中随时打开相关代码进行阅读。
• 评审核心代码时，若是发现严重问题，应马上终止 CR，找 MR 提交者当面讨论。
• 若是 MR 提交者对评审意见提出异议，评审者应找提交者当面讨论，避免在评论区互踢皮球。
• 合并代码以前应确保全部评审意见都被妥善处理。
• 记得点赞。

4 小结

2019 年 Stack Overflow 的一份调查报告显示，超过 7 成的程序员会把 CR 当作平常工做的一部分，近 1/3 的程序员每周在 CR 上花费 2~3 个小时，还有 1/3 的程序员每周花费 4~5 个小时。内心默默算一下，你是在拖后腿仍是领路者？若是你还没作过 CR，那么赶忙行动起来；若是你已经在 CR，很好，请继续保持。一花一世界，一叶一菩提，码中自有乾坤。CR，走起！

5 参考

• 如何完成一次高质量的CODE REVIEW?（2020版）
• 陈皓：CODE REVIEW中的几个提示
• 陈皓：从CODE REVIEW 谈如何作技术
• 如何作有效的代码审查？我有这些建议