Question

“代码审查是件很无聊的事情，费时费力，纯粹形式主义”，有这样想法的开发人员不在少数，甚至很多团队由于这些原因或者开发进度等其他因素干脆就直接忽略这个环节。那么，是不是代码审查真的那么一无是处呢？真相是：它远比你想象的要重要得多。很多人之所以会产生这样的误区，多半是因为代码审查的流程不够高效或者审查的目的出现了偏差。我们通过一组统计数据来看严格高效的代码审查到底有多重要，如图7-6所示。

图7-6　不同代码审查效率下修复bug的代价

图7-6所表示的是在不同的阶段中未经过严格有效的代码审查和经过高效代码审查后所得到的bug发现比率和修复代价对比关系。从图7-6可以看出，有效的代码审查流程非常必要，它能够使得大量bug在该阶段被发现，并且大幅度降低bug修复的总体代价，因为代码审查阶段bug的修复代价非常小。除此之外，代码审查还有助于建立高效的团队，提高团队成员之间的协作能力以及编码水平，有助于跨团队之间知识共享。那么，团队应该以什么样的态度去看待代码审查呢？

1）不要错误地理解代码审查会的目的。代码审查会的首要目的是提高代码质量，找出defect或者设计上的不足而非修复defect。也许有人会疑虑，defect不要修复吗？要！但绝不是在审查会上，这是保证代码审查高效的第一个关键，因为时间有限，不可能也不允许在审查会上去讨论bug或者defect如何修复，所有类似问题应该记录下来等会后讨论。

2）代码审查过程不应该有KPI（Key Performance Indicator）评价的成分。如果将代码审查中发现bug的概率作为对开发人员绩效考核的标准，势必会打击参与人员的积极性，有些人可能因为怕承担责任而不愿意直接指出代码中的问题。同时也会引起被审查人员的自我保护意识，当问题发生的时候，被审查者可能更关注于怎么推卸问题而不是静下心来解释问题产生的原因，以及如何改进。因此，代码审查要努力做到针对技术和问题本身。

3）对管理层的建议：除非经理或者组长真正参与到具体的技术问题，否则应该尽量避免这些人员参与代码审查会，因为这些人直接与员工KPI评价挂钩，即使申明不会用bug或者defect发现的数目来作为评价标准，但由于这些人身份的特殊，仍然可能造成评审参与人员不能诚实面对代码本身的局面。

4）对开发者的建议：把代码审查当做一个学习的机会，而不要看成是浪费你的时间帮别人来解决问题。无论你技术水平的高低，阅读和审查同行的代码可以让你学习更优秀的设计和编码，也能让你更快速地知道如何避免一些糟糕的代码和设计上的缺陷。

有了正确的态度，还需要一定的流程来保证才能做到高效的代码审查。

（1）定位角色。

一般来说代码审查会上有4类角色：仲裁者、会议记录者、被评审开发人员和评审者。

1）仲裁者：也叫主持人，一般由技术专家或者资深技术人员担任。担任该角色的人员不仅要技术精湛、业务精通，而且要有全局系统思维和较好的沟通以及领导能力。他的主要职责是控制会议流程和时间，保证会议流程的高效性，同时能够在必要的时候给予评审技术指导。因此在评审过程中如果出现了参与人员就一个问题争论不休的时候，该角色应该能够给予及时制止和指导性意见，特别要注意保护被评审人员。在评审结束之后，仲裁者还应该确保发现的问题被正确解决。

2）会议记录者：及时记录评审过程中发现的问题，包括问题的提出者、问题描述、问题产生的位置，如果有解决思路还应该记录解决方法，如果问题暂时没有解决办法应该记录下一步行为，如是会后研究还是另外开会讨论等。需要强调的是，在记录完一个问题相关信息之后，会议记录者必须和被评审开发人员确认记录内容，以保证记录的信息准确无误且被该开发人员认可。

3）被评审开发人员：一般被评审开发人员在评审开始的时候应该对其代码有综合性的介绍，在评审者提出问题或者质疑的时候应该及时给出解释。被评审开发人员对其他人提出的意见或者问题要正确看待，不要当做攻击，记住：所有人的目的都是为了开发出质量更高的软件。被评审开发人员一般不参与对代码的具体审查。

4）评审者：除了以上3类人员外，其余与会的人都称为评审者，评审者组成应该是有经验的和经验相对较弱的人员兼而有之，因为这样的组合才能在评审的同时更好地起到培训作用，提高经验较弱人员的编码能力。

（2）充分准备

在代码提交审查之前，代码作者需要做以下准备：对代码进行自我修正，包括对代码风格进行检查、添加必要的注释、完成必要的功能测试等；提前通知参与审查的人员，以便他们能够事先对代码框架有个基本了解；确定会议时间和进程。

图7-7　台面检查示例

（3）合理使用技术和工具

代码审查并不是完全依靠经验的，有一些方法可以遵循。常见的方法和工具如下。

①检查表（checklist）：检查表有利于有针对性地发现代码中存在的问题，如变量是否初始化，函数调用的参数，命名是否一致，字符串是否正确解码，有没有import未使用的lib，逻辑操作符是否正确，（）、{}对是否一致等。

②台面检查（Desk Checking）：适合在编码早期对顺序执行的代码进行检查，手工模拟代码的执行过程来检查程序中潜在的问题。图7-7所示就是下面的代码的台面检查。

i = 0
j = input('num:')
sum = 0
if i < j:
   sum = i+j
else:
   sum = j

③IRT（Interleaving Review Technique）：适合于并发性的代码或者容错性系统。更多IRT的资料读者可以参考https://www.research.ibm.com/haifa/Workshops/PADTAD2004/papers/irt_padtad2.pdf。

④代码审查工具：如Rietveld、review board、Collaborative Code Review Tool（CCRT）等。

（4）控制评审时间和评审内容

为了保证效率，一般来说一次评审时间要尽量控制在45分钟到1小时。研究表明，当人的注意力集中超过1小时，效率就会急剧下降。而一次评审的代码行数应控制在200行以内，最好不超过400行。如图7-8所示描述的就是缺陷发现的密度与评审代码行数之间的关系。由图可知当被评审的代码超过200行时，查出缺陷的密度就会急剧下降。

图7-8　代码行数与查出缺陷密度之间的关系

（5）关注技术层面，对事不对人

要把重点放在技术问题以及如何解决上，而不是诸如代码风格（代码风格当然重要，但应该在评审之前就由开发者对照组织规定事先完成，评审过程附带指出一些特殊问题）、时间进度之类的非技术层面的问题上。此外，评审人员应该对事不对人，在评审过程中要合理使用一些客观的语言，如“我没读懂这段代码的意思”，或者“我认为怎样做更好”、而不是“你这代码写的太烂了”、“垃圾”等带有攻击性的语言。较好的方法是评审者在评审的过程中时刻警惕：代码是不是正常工作？与需求是不是匹配？实现上有没有潜在的问题和风险？推荐采取“六顶帽子”思考法。

（6）记录问题，追踪进一步行动

记录问题是为了保证在评审会上发现的问题和缺陷在会后能得到及时的修复。因此会议记录者应该在会后及时将会议记录发送给相关人员，并保证后续行动都及时实施。

（7）不要忽视附加的培训作用

评审是手段，发现代码缺陷，提高代码质量和团队人员的编码水平才是目的，因此评审过程中别忘了培训的附加作用，仲裁者应该针对优秀的代码鼓励其他参与者借鉴，而对于一些警示问题代码要提醒参与者避免。