Google 代码评审经验借鉴（2）-- 代码评审清单

在考虑每一点时，请务必考虑代码评审标准。

01. 设计

在评审中最需要要关注的就是修订的整体设计。修订中各段代码的交互有意义吗？此更改是否属于基础代码，还是属于库？它是否与系统的其它部分良好集成？现在是添加此功能的良好时机吗？

02. 功能

该修订是否实现了开发人员的目标？开发人员的目标是否有益于代码的用户？用户通常是（受更改影响的）最终用户和（未来使用代码的）开发人员。

多数情况下，我们期望开发人员在进行代码评审前能够充分测试修订，以确保它可以正常工作。尽管如此，作为评审人员，你仍需要考虑边缘情况，查找并发问题，尝试像用户一样思考，并确保评审完成后仅阅读代码找不到任何错误。

如有需要，评审人员可以自行验证修订，尤其当修订的行为具有面向用户的影响时（如 UI 更改）。因为此时只阅读代码，难以识别某些更改对用户具体会产生怎样的影响。对于此类更改，如果评审人员不便自行尝试，可以邀请开发人员为你提供功能演示。

此外，在修订中是否存在并行编程，且在理论上有机会导致死锁或竞争条件，在评审时当予以重视。此类问题在运行代码时难以识别，需要有人（开发人员和评审人员）仔细考虑并行逻辑的设计，以确保不会引入其它问题。（请注意，一般不推荐使用可能存在竞争条件或死锁的并发编程，因为不利于代码评审和代码理解。）

03. 复杂性

修订的编码是否比其本该复杂？在修订的各个层级检查此项，单行复杂了吗？函数复杂了吗？类复杂了吗？复杂是指过度设计，一般意味着代码的读者难以快速理解。它还可能意味着开发人员在尝试引用或修改此代码时容易引进缺陷。

有一种特殊的复杂性是过度工程化，如：开发人员过分追求代码的通用或添加当前阶段系统使用不上的功能。评审人员要对过度工程化保持警惕。鼓励开发人员解决现在需要解决的已知问题，而不是实现悬而未决的问题。未来的问题未来解决，届时你弄清楚它的面貌，更知道如何设计。

04. 一致性

如果没有其它约束规则，则作者应与现有代码的设计或风格保持一致。我们往往能够识别显著的一致性冲突，却往往会忽视小的非一致性更改。系统恰恰就会因为那些小的更改（违背基础代码在设计上的一致性的更改）而变得异常复杂，因此，防止此类更改也尤为重要，代码中特例越少，越有利于理解与维护。

无论哪种方式，鼓励作者开 Bug 或添加用于整理现有代码的 TODO。

05. 命名

开发人员有尝试为一切开发元素取个好名字吗？好的名字，足以传达项目（item）是什么或做什么，而刻意简化，会使代码难以阅读和理解。

06. 注释

开发人员写了有助于理解代码的注释了吗？某条注释实际上又是否必要？一般情况下，有效的注释应是解释某段代码为什么存在的注释，而非解释某段代码正在执行什么。

如果代码不够清晰（或元素命名不够清晰准确，或复杂性的问题没处理得当），无以自我说明，是否说明代码本身可以简化了？尽管有一些例外（如正则表达式可以注解在做什么操作），但多数注释当以提供代码本身无法说明的信息为己任，如决策背后的推理。

在修订之前，查阅已经存在的注释也很有帮助。也许有一个 TODO 可以删除，也许有一个注释的建议反对你的更改，等等。

07. 风格

如果现有代码与风格指南不一致，该怎么办？根据我们的代码评审原则，风格指南具有绝对权威：如果风格指南有要求，修订就要遵守。

假设某风格指南的条例更像是建议而非要求，则需主观判断新的代码是应该与风格指南一致还是与周围代码一致。请优先奉行风格指南，除非它使新增的代码显得过于突兀。

如果评审人员想改进风格指南中未说明的一些风格，请以”NTH：”作为评论前缀，让开发人员知道，你认为这在某种程度上会改进代码，但非硬性要求。不要根据个人偏好阻止修订的获准。

作者不应将（与解决问题无关的）风格更改与其它更改混合，使得评审人员难以识别修订中正在更改的关键所在，进而使代码的合并和问题的回滚更加复杂。例如，如果作者或评审人员想要重新格式化整个文件，请另起一个修订专门处理此事，然后再发送另一个修订功能相关的更改。

08. 测试

要求针对更改进行单元测试或集成测试。测试要添加到与产品代码相同的修订中，除非修订是为了处理紧急情况。

确保修订中的测试正确合理有效。测试向来不会自测，我们基本不会为了测试本身再写测试，所以我们必须确保测试行之有效。当代码有缺陷时时，测试是否会报错？如果代码发生更改，它们是否会误报？每项测试是否都有简单而有用的断言？测试基于测试方法划分得当吗？

请记住，测试也需要便于维护。不要仅仅因为测试并非主程序的一部分而接受测试的复杂性。

09. 文档

如果修订更改了用户构建、测试、交互或发布代码的方式，请检查它是否还更新了关联的文档，含 README、g3doc 页和任何生成的参考文档。如果修订删除或弃用代码，请考虑是否应删除文档。如果文档缺失，请要求补充。

10. 每一行

请查阅待评审的每行代码。某些内容（如工程文件）有时可以快速浏览，但人工编写的代码（库，类，代码块等）不可以快速浏览，并假定其中的内容没问题。显然，有些代码比其他代码更值得仔细审查，这基于你的判断，但至少你应该确保你理解了的你所评审的代码在做什么。

如果阅读代码难以读懂，并且会明显降低评审速度，则应首先知会开发人员，等待他们澄清，然后再尝试评审。在公司，我们聘请的都是优秀的软件工程师，而你就是其中之一。如果你都无法理解代码，则其他开发人员极有可能如此。因此，当你要求开发人员澄清该代码时，你还帮助未来的开发人员理解代码。

如果你了解了代码，但你觉得没有资格对部分内容进行评审，请确保修订上有一个合格的评审人员，尤其是诸如安全、并发、可访、国际化等复杂问题。

11. 上下文

一般来说在文本的上下文中思考修订才有帮助。通常，代码评审工具只显示更改周围数行代码。有时，你需要阅读完整文件，以确保更改有意义。例如，你可能只看到新增了四行代码，但当你审阅完整文件时，你会发现那四行代码位于一个 50 行方法中，现在确实需要那四行代码以解决某一问题或完善代码功能。

12. 典范

如果你在修订中看到一些好的东西，请告诉开发人员，尤其是当他们以出色的方式处理你的评论时。代码评审不应关注错误，还应该提倡并赞赏优秀的实践。在指导方面，有时告诉开发人员做对了什么比告诉他们做错了什么更有价值。

13. 要点汇总

是否与原有系统良好集成？
是否对周围环境有副作用？
是否解决了问题？
是否有益于用户？
UI的更改是否合理美观？
并行编程是否安全可靠？
是否比其本该复杂?
是否在尝试解决悬而未决的问题？
是否有违基础代码的一致性？
每个元素命名清晰准确吗？
注释是否清晰有效，基本解释了为什么而非是什么？
是否遵守风格指南？
测试是否合理有效？
测试是否便于维护？
关联的文档有没有正确地增删或更新？