如果您有过代码审查的经验,一定经历过高频、耗时且繁琐的拉取请求、提交确认、以及批准等过程。为了避免凌晨5点,您还在孤独地审查着从团队分配来的代码,我们走访了该领域的专家与用户,收集到了如下小贴士,以方便您快速高效地完成任务。
1、审查较小部分
通常而言,更小的提交会导致更小、且更易于管理的代码审阅。人们往往需要花较长的时间,去理解由大块代码的修改所涉及到的大量依赖项。而小块的修改不仅能够节省代码审查的时间,而且能够减轻审查人员的认知负担,方便审查过程的流畅开展。可以说,将工作量分成较小的部分,往往可以使团队成员加深对于变更意图与方式的了解。
有调查表明:一次性审查约行代码的效果是最佳的。否则,过多的代码量,会直接导致审查者发现问题能力的下降。当然,我们也可以根据实际使用的平台、以及开发的语言,略作调整。如今大家都在谈论“长期主义”。一开始,大家可能会不太适应这种较小的审查提交方式,但是如果能长此以往地坚持下去,它的好处将非常明显。
2、时限要求
与代码测试相比,代码审查环节往往被要求在有限的时间内完成并提交。为了能够及时地发现代码本身、及其架构和时序等相关问题,审查人员往往需要对代码具有充分的了解。因此,在这种“时间紧、任务急”的情况下,团队需要对待代码的审查重点排定优先级。例如,我们可以针对如下方面,根据实际情况进行审查:
条件判断语句或其他逻辑是否合理?
代码中的字符串是否已格式化?
是否在不必要之处使用了同步或锁的操作?
是否采用了原子变量?
是否使用了不必要的线程,且是否安全?
数据结构是否安全且高效?
根据上述要点,我们整理出一份审查清单(下文将会提到),以方便团队合理地安排时间与精力。当然,即使在项目进度紧迫,无法进行完全代码审查的情况下,我们也需要保证新的、关键性的、具有依赖关系的代码部分,能够被审查到。
3、对事不对人
代码审查讲求的是以”对事不对人”的态度,全面提高代码的质量。例如:倘若甲发现了乙的代码中存在着问题,这并不意味着甲的编程能力强,而乙的能力弱。我们更不能据此去惩罚乙(当然,奖励甲是应该的)。否则,甲可能因为害怕破坏了与团队中其他成员的关系,而不愿在审查中指出真实潜在的问题,进而导致代码的审查效果失真。
此外,有些团队会将代码审查的任务集中在某个专人身上。这样除了会造成人员安排上的单点风险以外,还会造成其工作量上的繁重和认知度上的局限性。
4、共享编码与审查准则
既然不合适将代码审查任务固定地托付给一个人,那么我们势必要构建一个团队或小组。而为了保证集体智慧“在线”,我们需要让所有参与者都事先了解相关的审查准则。例如:
规范化针对代码添加的注释,阐明代码修改的原因,以方便后续者的解读。
修正编码风格,尤其是一些关键性的数据结构、以及方法的命名规则。
审查是否充分考虑到了变量或异常可能出现的所有情况。
当然,对于不同的编程语言与平台,我们可以采取略有不同的编码与审查准则,并记录在案。下面是在社区里被广泛采用的代码样式标准。它们可以促进和加速新的成员,在代码风格上尽快地融入团队。
Ruby
Community-driven
Github’sstyle
SOAnswer
Rubydoc
Javascript
Crockford’sguide
Felix’sNodestyle
Mozilla
Idiomatic.js
Python
Guido’sPEP8
python-guide.org
PHP
Pear
PHP-FIG
PSR-0:AutoloaderStandard
PSR-1:BasicCodingStandard
PSR-2:CodingStyleGuide
5、审查清单
常言道,“按图索骥,效率最高”。我们可以参考如下清单,来逐条检查并提高代码审查的流程与效率。
编程习惯方面
是否删除了每行代码尾部的多余空格。
是否存在从未被用到的变量。
是否针对变量、方法、以及类采用了相同的命名规则。
对于括号、循环、if语句等是否采用了统一的格式。
是否将常量写在了独立的常量类中。
是否针对不同的异常(exception),采用了不同的捕获(catch)语句。
是否避免了单个方法过于冗长。
为了避免一条语句过长,且超过编辑工具的可视区域,是否按需对其进行了拆分。
是否给各种方法添加了适当的访问控制,而非一律采用public。
是否合理地用到了静态工厂方法,而非重载构造函数。
功能方面
如果某个逻辑会被多次使用到,那么就应该将它写成帮助类或是API,以便被频繁调用。
不同语言的代码不应被相互混用。例如,在JSP中就不应包含Java代码。
安全方面
任何代码在未经转义之前,都不应直接执行用户的输入。例如JavaScript中的eval函数,以及SQL语句。
在代码级别上,禁止那些在较短时间内,被大量提交的IP请求。
是否为每个类、变量、以及方法都设置了正确的访问域。
性能方面
是否能及时地关闭不需要的数据库实例、以及文件的操作句柄。
SQL语句的用法是否规范。
是否按需创建了immutable的类。
是否避免使用了heavy对象。
是否将全局信息保存在“applicationcontext”中。
文档方面
是否说明了代码中的每一个类、方法、及其基本逻辑。
是否提供了针对复杂的HTML、JavaScript、以及CSS等相关文档说明。
是否为代码文件的头部添加了相关的版权信息。
如果是对某个缺陷的修复,是否分配了对应的缺陷ID。
6、使用自动化协同审查工具
在处置Git存储库里的频繁提交等场景时,光靠一双眼睛式的人工审查方式,显然是非常低效且耗时的。为了发挥团队的优势,我们需要引入协同审查工具,以实现更快速、更自动化、更全面的合作式审查。目前,Github、Bitbucket和GitLab都能够通过拉取式请求的特性,提供内置的代码审查工作流。当然,我们也可以用到诸如:CheckStyle、PMD、FindBugs、Upsource、以及Codacy等工具。
7、使用短路
如果您时间非常紧迫,并且无法去逐行检查代码的内容,那么请采用“抓大放小”渐进的方法。毕竟,那些在函数调用时可能引起缓冲区溢出的问题,以及在生产环境中会导致程序崩溃的潜在缺陷,比起一眼可以看出的代码样式问题,要重要得多。您可以在代码检查中,插入所谓短路(short-circuiting)元素,以事先处置和纠正重大的问题,而将较小的缺陷,留待将来时间充沛时,再予以更正。
8、提交与确认
从某种意义上说,代码审查旨在提高开发者的编程能力,让其从自身犯过的错误中吸取教训,从他人的修改思路中学习进步,而应当避免其产生抵触或反感情绪。
值得注意的是,代码审查人员不应以交付时限为由,针对其发现的问题,直接对代码进行修改,甚至予以重构,而应当提请代码开发人员进行确认。此方法不但可以确认问题是否真实存在,并共同获取最佳解决方案;而且可以让代码作者深入了解其自身的不足,协助其不断进步。