【转】如何作人性化的代码审查？从高到低、用例子

（点击上方公众号，可快速关注）

 

编译：精算狗，英文：Michael Lynch

 

最近，我一直在读有关代码审查最佳范例的文章。我注意到这些文章的关注点是找到 bug，而忽略了代码审查其余的部分。用建设性、专业的问题沟通方式？不相关！只要识别出全部的 bug，剩下的部分会水到渠成。

 

我只能假设我读过的这些文章都来自将来，那时候全部的开发人员都是机器人。在那个世界，你的队友欢迎对其代码未通过推敲措辞的批评，由于处理这样的信息能温暖他们冰冷的机器人之心。

我要作一个大胆的假设，你想要在当前世界改进代码审查，此时你的队友都是人类。我还要作一个更大胆的假设，你与同事之间积极的关系自己就是一个目的，而不只仅是一个可调整的变量来最小化缺陷的平均成本。在这些状况下，你的审查实践会发生怎样的变化呢？

在这篇文章中，我讨论了一些技巧，把代码审查既看做是技术过程，也看做是社会过程。

什么是代码审查?

“代码审查（code review）”这一术语能够指一系列活动，从简单地站在队友身后读读代码，到 20 人与会的单行代码分析。我用这一术语指正式的、书面的过程，但也不像一系列现场代码审查会议那么重大。

代码审查的参与者包括做者以及审查者：做者写代码并把代码送去审查，审查者读代码并决定代码何时就绪并入团队的代码库。一次审查能够由多个审查者完成，可是我作了简化的假设——你是惟一的审查者。

在代码审查开始以前，做者必须建立一个变动表。做者想要将源代码并入团队代码库，变动表包括一系列源代码的变动。

看成者把变动表发给审查者时，审查就开始了。代码审查是循环发生的。每一个循环都是做者与审查者之间完整的往返：做者发送变动，审查者给予变动的书面反馈。每次代码审查都包括一次或者更多的循环。

当审查者批准了这些变动，审查结束。这一般指的是给出 LGTM，“我以为不错（looks good to me）”的简写。

这为何很难？

若是程序员给你发了一份变动表，他们以为这个变动表棒极了。你又给他们写了一份详细的清单，解释为何这个变动表并很差。这是须要当心处理的信息。

这是我不想念 IT 的一个缘由，由于程序员是很是不可爱的人……好比，在航空业，那些过度高估了本身技术水平的人都死了。

Philip Greenspun，ArsDigita 的联合创始人，引自《Founders at Work》。

做者容易把对其代码的批评解读为暗示他们不是合格的程序员。代码审查是一个分享知识和作工程决定的机会。可是若是做者把讨论理解为我的攻击，这个目标没法达成。

除此以外，你还面临着书面传达想法的挑战，词不达意的风险会更高。做者听不到你的语气，也看不到你的肢体语言，因此清晰地、当心地传达你的反馈更为重要。对一个有戒备心的做者来讲，一句无冒犯意味的批注，好比“你忘了关闭文件句柄”，能够被理解成“真不敢相信你忘了关闭文件句柄！你真是个傻子。”

技巧

1. 让电脑作无聊的部分

2. 用风格指南平息风格争论

3. 立刻开始审查

4. 从高级别开始，逐步向下

5. 慷慨地使用代码示例

6. 永远别说“你”

7. 把反馈表达成请求，而不是指令

8. 把批注与原则联系在一块儿，而不是观点

让电脑作无聊的部分

在会议和邮件的干扰下，可用来专一于代码的时间不多。你的精神毅力更是短缺。读队友的代码是认知上的负担，要求高强度的专一。别把这些资源浪费在电脑能作的任务上，尤为是当电脑能作得更好的时候。

空白错误是一个显著的例子。比较一下人类审查者找到缩进错误并与做者一块儿改正所花费的精力，和仅仅使用一个自动排版工具所花费的精力：

人类审查者须要的精力	排版工具须要的精力
1.审查者寻找空白错误，找到错误的缩进   2.审查者写批注，指出错误缩进 3.审查者从新读批注，确保措辞清晰，不含指责意味 4.做者读批注 5.做者改正代码缩进 6.审查者核实做者适当地处理了批注	无！

右边是空的，由于做者用了一个代码编辑器，每次他们点击“保存”时，该代码编辑器会自动规定空白的格式。在最糟的状况下，做者把代码发出去以供审查，持续集成解决方法报告说空格错误。做者在不须要审查者顾虑的状况下，修正这个问题。

在代码审查中寻找能够被自动解决的机械性任务。如下是常见的例子：

任务	自动解决方法
验证代码的构建	持续集成方法，好比 Travis 或者 CircleCI
证明经过了自动测试	持续集成方法，好比 Travis 或者 CircleCI
验证代码空白与团队风格一致	代码排版器，好比 ClangFormat (C/C++ 排版器) 或者 gofmt (Go 排版器)
识别未使用的输入或者变量	代码 linter，好比 pyflakes (Python linter) 或者 JSLint (JavaScript linter)

自动化使你做为审查者能作出更多有意义的贡献。当你能忽略一整个类别的问题，好比输入的排序或者源文件命名的约定，你可以关注更有趣的事情，好比函数错误或者可读性缺陷。

自动化也能给做者带来好处。自动化使做者用几秒钟发现粗心的错误，而不是几小时。即时反馈使得从错误中学习更容易，修正错误的代价也更小，由于做者脑海中还有相关的背景。另外，若是他们不得不听到本身犯下的愚蠢错误，对自尊心来讲，从电脑那听到要比从你那听到更容易被接受。

和你的团队一块儿将这些自动检查加入代码审查的工做流程中（例如，在 Git 中的 pre-commit hooks 或者 Github 中的 webhooks）。若是审查过程要求做者手动运行这些检查，你会损失大部分好处。做者老是会忘记一些状况，迫使你继续审查简单的问题，而这些问题原本就能被自动处理。

用风格指南平息风格争论

关于风格的争论浪费了审查的时间。一致的风格确实重要，可是代码审查不是争论花括号位置的时候。在审查中消除风格争论的最佳办法是，遵照一个风格指南。

好的风格指南不只定义了像命名习惯或者空白规则这样的表面元素，并且定义了怎样使用给定编程语言的特征。好比，JavaScript 和 Perl 都包含了一些功能——他们提供了许多实现相同逻辑的方法。风格指南定义了作事的惟一方法，这样不会以一半队员用了一组语言特征而另外一半队员用了彻底不一样的一组特征收尾。

一旦有了一个风格指南，你就不须要浪费审查循环，来跟做者争论到底谁的命名习惯最好。只要听从风格指南而后继续就行。若是你的风格指南没有指定某个特定问题的约定，那它通常都不值得争论。若是你遇到一个风格指南未涉及的问题，它又重要到须要讨论，和团队一块儿推敲。而后把决定加到风格指南，这样大家永远不须要再进行一次这个讨论。

选择 1：采纳一个现存的风格指南

若是从网上搜索，你能找到已发布的风格指南可供使用。Google 的编程风格指南是最知名的，可是若是它的风格不适合你，你能够找到其余的指南。经过采纳一个现存的指南，不须要从头创造一个风格指南的大量花费就能继承其好处。

坏处是组织为他们本身特别的须要优化其风格指南。好比，Google 的风格指南在使用新语言特征上比较保守，由于他们有一个巨大的代码库，其中的代码要在全部东西上运行，从家用路由器到最新的 iPhone。若是大家是一个只有一个产品的四人小组，你可能选择在使用前沿语言特征或者扩展时更大胆。

选择 2：不断创造你本身的风格指南

若是你不想采纳现存的指南，你能够本身创造一个。在代码审查中每产生一次风格争论，向整个团队提问来决定官方约定应该是什么。当大家达成共识，把决定编进风格指南中。

我倾向于将团队的风格指南做为源控制下的 Markdown（例如 GitHub 页面）。这样，对风格指南的任何改动都须要经过普通的审查过程——某人得明确批准改动，并且团队中的每一个人都有提出疑虑的机会。Wikis 和 Google 文件都是可接受的选择。

选择 3：混合方法

合并选择 1 和选择 2，你能够采纳现存的风格指南做为基础，而后用本地风格指南来扩展或者覆盖这个基础。一个好例子是 Chromium 的 C++ 风格指导。它用 Google 的 C++ 风格指导做为基础，可是在其上添上本身的改动和附加。

立刻开始审查

将代码审查视为高优先级。当你真正阅读代码并反馈时，慢点来，可是要立刻开始审查——最好在几分钟内开始。

若是队员发给你一个变动表，这可能意味着直到你完成审查前，他们会卡在其余工做上。理论上，源控制系统使做者能建起新的分支，继续工做，而后从审查中把变更合并进新分支。实际上，一共有大约四个开发者可以高效地作这件事。其余人要花很长时间来清理三方差别，以至于抵消掉了等待审查完成这段时间里的进步。

你立刻开始审查，就创造了一个良性循环。你的审查时间彻底变成了一个与做者的变动表大小和复杂度相关的函数。这激励做者发送短小、范围狭窄的变动表。对你来讲这样的变动表审查起来更容易，也更愉悦，因此你能更快地审查，循环继续。

想象一下你的队员要执行一个新特征，这个特征要求 1000 行代码变动。若是他们知道你能在大概 2 小时内完成一个 200 行的变动表的审查，他们能够把特征拆分红各包含 200 行的变动表，而后在一两天内检查完整个特征。可是，若是不管大小你都要花一天来完成全部的代码审查，如今就要花一周时间才能检查完整个特征。你的队员不想傻坐一周，因此他们被激励着去发送更大的代码审查，好比每一个包含 500 到 600 行。这样审查起来花销更大，反馈也更差，由于记 600 行变动表的背景要比 200 行变动表难。

一个审查循环的最大周期应该是一个工做日。若是你正在处理一个更高优先级的问题，不能在一天内完成一个审查循环，让你的队员知悉并给予他们把审查交给别人的机会。若是你一个月被强制回绝审查超过一次，可能意味着你的团队须要放慢脚步，这样你能保持理智的开发实践。

从高级别开始，逐步向下

在一个既定的审查循环中，你写的批注越多，让做者感受受打压的风险越大。准确的界限随开发者的不一样而不一样，可是一个审查循环中 20 到 50 个批注通常是危险区的开始。

若是你担忧把做者淹没在批注的海洋里，约束你本身在早期循环中反馈高级别的问题。注意从新设计类接口或者拆分复杂函数这样的问题。等到这些问题都解决了再去处理低级别的问题，好比变量命名或者代码评论的清晰度。

一旦做者整合了你高级别的批注，低级别的批注可能会变得无心义。把低级别的批注推迟到后期的循环中，你能够把本身从当心措辞的工做中解救出来，也省得做者处理没必要要的批注。这个技巧也细分了审查过程当中你所关注的抽象层，帮助你和做者用清晰、系统的方法完成变动表。

慷慨地使用代码示例

在一个理想的世界里，代码做者会感谢收到的每一次审查。这是他们学习的一个机会，也能防止他们犯错。事实上，有许多外部因素能致使做者负面地解读审查，怨恨你给他们批注。可能他们正面临着截止日期的压力，因此除了马上不经审查的批准之外的东西都感受像阻碍。可能大家没怎么在一块儿工做过，因此他们不相信你的反馈是好意的。

一个让做者对审查过程感受良好的方法是，在审查中找机会送他们礼物。全部开发者都爱收到的礼物是什么呢？固然是代码示例啦。

若是经过写一些建议的改动来减轻做者的负担，就证实了做为审查者，你对时间很慷慨。

好比，想象一下你的一个同事不熟悉 Python 的列表推导（list comprehension）特征。他们给你发送了包含如下代码的审查：

urls = []

for path in paths:

  url = 'https://'

  url += domain

  url += path

  urls.append(url)

 

回复“能用列表推导（list comprehension）简化这个吗？”会使他们苦恼，由于如今他们得花 20 分钟搜索他们以前从没用过的东西。

收到像如下这样的批注他们会更开心：

考虑用像这样的列表推导（list comprehension）来进行简化：

urls = ['https://' + domain + path for path in paths]

 

这个技巧并不局限于单命令程序。我会常常创建我本身的代码分支，向做者展现概念的一个大型证实，好比拆分一个大型函数或者增长一个单元测试来覆盖一个附加边界状况。

为清晰、无争议的改进保留此技巧。在上面列表推导（list comprehension）示例中，极少有开发者会拒绝减小 83% 的代码行数。相反，若是你写了一个冗长的示例来演示某个变更“更好”，而这个变更是基于你本身的我的品味（好比，风格变更），代码示例让你看起来执拗己见，而不是慷慨大方。

限制你本身在每一个审查循环中只写两到三个代码示例。若是你开始为做者写整个变动表，这标志着你以为做者没能力写本身的代码。

永远别说“你”

这听起来挺怪异的，可是听我说：永远别在代码审查中使用“你”这个字。

在审查中作的决定应该是基于什么能让代码更好，而不是谁出的主意。你的队员在他们的变动表中倾注了大量心血，并且极可能为本身的工做感到骄傲。他们听到对其工做的批评，天然反应是摆出防护和保护的姿态。

组织反馈所使用的措辞，以最小化激起队员戒备心的风险。讲清楚你是在批评代码，而不是程序员。看成者在评论中看到“你”这个字，会将他们的注意力从代码转移到本身身上。这增长了他们把批评私人化的风险。

考虑一下这个无害的评论：

你拼错了“successfully”。

做者能够把这个批注理解成两种不一样的意思：

• 理解 1：嗨，好家伙！你拼错了“successfully”。可是我仍是以为你聪明！那可能就是个笔误。

• 理解 2：你拼错了“successfully”，笨蛋。

把这个跟省略了“你”的批注比较一下：

sucessfully -> successfully

后者是一个简单的修正而不是对做者的审判。

幸运地是，在从新写反馈时避免使用“你”并不难。

选择 1：用“咱们”替换“你”

你能重命名这个变量，让它更具备描述性吗？好比 seconds_remaining。

变成：

咱们能重命名这个变量，让它更具备描述性吗？好比 seconds_remaining。

“咱们”增强了团队对代码的集体责任。做者可能跳槽到一个不一样的公司去，你也可能，可是拥有这个代码的团队会一直以不一样的形式存在。当你明显指望做者本身作某些事的时候，说“咱们”听起来会比较傻，可是傻要比指责好。

选择 2：移除句子的主语

另外一个避免使用“你”的方法是用省略句子主语的简化句子：

建议重命名为更具备描述性的名称，好比 seconds_remaining。

你能够用被动语态实现类似的效果。我在技术写做中通常会避免像瘟疫同样使用被动语态，可是它是个有用的方法来避免使用“你”。

变量应该被重命名为更具备描述性的名称，好比 seconds_remaining。

另外一个选择是把它表述为一个问题，用“……如何”或者“……怎么样”开头：

把变量重命名为更具备表述性的名称怎么样？好比 seconds_remaining。

把反馈表达成请求，而不是指令

代码审查相对日常的交流来讲，要求更多的机智和谨慎，由于存在高风险把讨论转变成私人争论。你会指望审查者在审查中表示出礼貌，可是奇怪地是，我发现他们走向了另外一个方向。多数人永远不会对同事说“给我订书机，再给我拿瓶汽水。”可是我看到过无数审查者用相似的指令来表达反馈，好比，“把这个类移到一个单独的文件里。”

宁肯在反馈中恼人地绅士。把批注表达成请求或者建议那样，而不是指令。

比较用两种不一样方式表达的同一个批注：

表达成指令的反馈	表达成请求的反馈
把 Foo 类移到一个单独的文件里。	咱们能把 Foo 类移到一个单独的文件里吗？

人们喜欢掌控本身的工做。向做者提出请求给他们带来自主意识。

请求也让做者礼貌地反馈更容易。可能他们的选择是有合理的。若是把反馈表达成指令，来自做者的任何反馈都像违反指令。若是你把反馈表达成请求或者问题，做者能简单地回答你。

比较对话的好斗程度，取决于审查者怎么表达他们的初始批注：

表达成指令的反馈（好斗的）	表达成请求的反馈（合做的）
审查者：把 Foo 类移到单独的文件里 做者：我不想这么作，由于那样就离 Bar 类太远了。客户几乎总会一块儿调用他们。	审查者：咱们能把 Foo 类移到单独的文件里吗？ 做者：能够，可是那样就离 Bar 类太远了，以及客户通常会一块儿使用这两个类。你以为呢？

看看当你构建虚拟对话来证实观点把批注表达成请求而非指令的时候，对话变得多么有礼貌。

把批注与原则联系在一块儿，而不是观点

当你给做者写批注时，既要给出变动建议，也要给出变动的理由。“如今，这个类既负责下载文件，也负责解析文件。咱们应该依照单一责任原则，把它拆分红一个下载类和一个解析类。”这么说会更好，而不是说“咱们应该把这个类分红两个。”

让你的批注有原则性的立足点，这样能让讨论走向更积极的方向更有建设性。但你有一个具体的缘由，好比“咱们应该把这个函数写成私有函数，来最小化 public 借口类”，做者就不能简单地回复“不，我倾向于个人方法。”更确切地说，他们能够，可是由于你演示了改动如何知足目标，而他们只陈述了一个偏好，他们会看起来很傻。

软件开发既是艺术也是科学。你不可能永远都能用肯定的原则来明确表达代码到底哪里出了问题。有时候代码只是难看或者不符合直觉，不容易肯定为何。在这些状况下，解释你能怎么作，可是保持客观性。若是你说“我发现这不容易理解”，这至少是个客观的陈述；相反，“这莫名其妙”是一个价值判断，不必定适用于全部人。

尽量以连接的形式提供支持证据。团队风格指南的相关部分是你能提供的最佳连接。你也能够连接到语言或者库的文件。高票 StackOverflow 回答也行，可是离权威文件越远，你的证据变得越不稳固。

第二部分：即将上线

敬请期待其余小技巧，包括：

• 处理特别大的代码审查

• 识别给予表扬的机会

• 尊重审核的，以及

• 化解僵局

由 Samantha Mason 编辑。插图来自 Loraine Yow。感谢 @global4g为这篇文章的早期版本提供宝贵的反馈。

以为本文有帮助？请分享给更多人

关注「算法爱好者」，修炼编程内功