Amir Kirsh
C++ 语言功能强大——但也极其复杂，复杂性使其极易引发误解和过度复杂化。相比简单语言，C++ 中的程序错误难以发现——相比其他语言，生产环境中的 C++ 程序错误更难定位。
简而言之，需要谨慎处理 C++——甚至是用鹰眼那样锐利的目光进行评审。
本篇博文分为两部分。第一部分，我们将讨论更多代码评审的普遍情况。第二部分，我们将深入探讨具体的 C++ 代码评审话题，并为 C++ 代码评审创建检查表。
C++ 代码评审的重要性
无论使用何种语言进行编码，总是难以发现代码中的程序错误和普通错误，而我们可能从头到尾都没意识到自己的错误。他人评审不失为一种解决问题的好方法。
此外，两个人或更多人之间的代码讨论便足以提出新问题，更有助于明确重要问题。
上述内容适用于任何所使用的编程语言，但 C++ 的复杂性使其尤为重要。必须额外增加至少一个人来评审代码。
代码评审前的注意事项
提交代码以供评审之前，需要注意两件事。
其一是通过静态代码分析，并能解释任一所发现的警告，这些警告可能是误报，或是出于某种原因不应处理。静态代码分析工具种类繁多——包括开源工具与商业工具——有什么理由不使用呢？当然，假定不存在任何编译器警告，但在极少数情况下，如果确实有理由忽略编译器警告，也应提前备好解释（比如，编译指示声明上的注释，用以指示编译器忽略该警告）。
其二是测试代码。运行单元测试时，最好透彻检查全部代码，包括错误和边缘案例。测试能保证代码运行，缺少测试，代码评审则毫无意义。此外，代码评审中待评审项目包含测试。如果需要在代码评审后增加测试，确保测试失败后对代码进行的变更不会避开代码评审。
静态代码分析和执行良好的单元测试均可降低问题影响代码评审的机率，并能事先修复问题，但也不能保证它们就是万无一失的。
例如，如果从一开始就弄错需求，再好的测试或者静态分析都无法发现其中的问题。另一方面，代码评审考虑了需求，有助于在代码和测试场景中直观了解。这样就能发现需求与实际代码间的不一致。
SmartBear 最近一项调查显示，24% 的受访者表示代码评审是公司提高代码质量的首要途径。单元测试位列第二，静态代码分析稍显落后。这并不是说单元测试和静态代码分析不重要，可能是受访者认为代码评审有时会受到忽视，或者没有很好地执行。但显而易见的，单元测试和静态代码分析无法取代代码评审。
代码评审中的主要注意事项
代码评审的最终目标是找出代码错误或可改进之处。这包括：
- 代码是否符合需求
- 代码是否符合编码惯例与准则
- 逻辑错误、容易出错的代码、潜在程序错误与实际程序错误，包括并发问题
- 设计缺陷、效率低下与未来准备情况
- 可简化代码之处
- 是否良好执行单元测试，这也有可能发现程序错误
详述上述清单之前，先来看看代码评审存在的技术问题和执行代码评审的方法。
代码评审的类型
执行代码评审的方法有以下几种：
传统代码评审会议
主要利益相关者与编写待商议代码的开发人员进行会面。群组规模取决于所评审代码的重要性与复杂性。评审可能包括其他团队的开发人员、系统工程师、产品经理与测试工程师。有人认为非开发人员参加代码评审是浪费时间，但如果会议重点是复杂需求的实现方式，他们可能会有宝贵见解。也可能只需要开发人员和一个评审人员。
“速战速决”可以立即修复代码评审中提出的问题（例如，重命名变量和函数或添加简短注释）。其他问题也应记录在案，便于日后处理。代码评审末尾，应该决定是否有必要合并代码（如果有必要，需要修正次要注释），或者有必要再次提交代码以供二次评审。
需要注意的是，所述会议可通过线下或是线上方式进行。
常规做法是让团队中未编写代码的人介绍代码，仅在不清楚的情况下才由编码人员进行介绍。这有助于确保代码的可读性。另一方面，让编码人员自行介绍代码可能更有效率。两种方式都值得尝试，要看哪一种更适合团队情况。
优点：
- 讨论有助于提出潜在问题。
- 编码人员可对注释作出回应，当场解决其中问题，并助力针对其他注释进行的讨论。
缺点：
- 难以深入有待详细调查的问题（有可能线下进行，且无法在同一次会议中解决）。
- 耗费时间、效率低下，尤其是在大型群组中进行的时候——并非每一次讨论都与全体与会者有关。
- 需要选择一个全体利益相关者均能参与会议的时间段。
离线代码评审
开发人员完成对代码的变更，并通过源代码控制、拉取请求或其他代码评审工具，要求进行代码评审。评审人员提出的问题会在源代码控制或代码评审工具中作为公开问题。开发人员可修复问题或对其进行注释。全部修复和注释受批后，代码评审周期结束。
优点：
- 适用于远程工作者或分散在更广地区和不同时区的团队。
- 评审人员可根据自身情况随时进行评审。
- 每个评审人员都能关注到代码的不同之处。
缺点：
- 团队成员之间没有讨论和反馈。
- 即便开发人员对于公开问题有着简单答复，可以关闭该问题。那么，如果开发人员只是关闭了这个问题（因为对此已有良好答复），我们还是不知道公开该问题的人对此有何意见。
- 这可能是评审未能充分覆盖代码。
近距离评审
一旦开发人员完成代码，另一个团队成员就会在原开发人员的注视下——没错——对其进行评审，遍历任何错误并提出建议。这种方法中，代码评审就不那么正式了。
优点：
- 适用于小型团队，可非正式地当面完成评审工作。
- 尽可能在代码定稿前进行，这时编码人员对代码的印象尚且深刻，容易修复并调整。
缺点：
- 可能会忽视部分项目或代码，遗漏其评审工作。
- 许多情况下，这种评审会在单元测试前执行，因此不会评审代码的最终版本。
结对编程是否有代码评审的必要？
结对编程时，两名开发人员将在同一台机器上工作。一人编码，另一人提供建议和反馈，偶尔互换角色。这种方法是极限编程 (XP) 方法的一部分。适用于处于职业生涯早期的开发人员，但对全部开发任务来说效率不高。此外，在处理复杂问题时，结对编程大有裨益，可结合两名开发人员的精华思想，但往往发生在设计阶段。随后，实际编码工作可由单个开发人员处理。
问题是，结对编程是否有代码评审的必要？当然有！
结对编程的两名开发人员从同一个角度编写代码。他们可能会解释需求、思考设计，并以类似的方式分析问题。有必要引入外部观点——有人提出问题、指出问题，并考虑需求。这就是正式代码评审至关重要的原因。
总结：不要把结对编程当作是“已经把代码评审作为编程阶段本身的一部分”。后续可能还有麻烦。
代码评审工具
代码评审工具对于管理评审周期以及可能出现的注释和问题都很有用。一些团队认为它们有用；另一些团队则偏向使用简单文件或直接在源代码控制中公开问题。如果有工具辅助的代码评审适用于你的团队，那就选择一个可以实现轻松协作、跟踪变更和分享注释的工具——用于代码评审会议和离线代码评审。
以下是常用工具清单：
- SmartBear Collaborator（商业，链接为免费试用版）
- Gerrit（开源）
- Codestriker（开源）
- Atlassian Crucible（商业，链接为免费试用版）。
带着清晰简洁和明确的任务，以积极的语气，进行友好发问
确保以积极的语气，进行友好发问（而不是攻击、侮辱性问题和语气）。即使自认为知道答案，也要像不确定一样询问（“这个操作锁定了吗？需要锁定吗？”而不要：“我没发现锁定，是你的程序错误”！）。给予良好反馈（“好方法，干得漂亮！”）也是可以的（最好附带鼓励！）。
但当涉及到代码评审所产生的任务时，这些任务应该是清晰、简洁而明确的（“需要为更新缓存操作添加锁定机制”）。
代码评审期间的注释类型
代码评审期间有几种类型的注释，每种注释都应区别对待：
- 重命名——可在代码评审中讨论命名。毫无疑问命名是编码中最难的工作之一（不管有没有好的理由）。然而，命名对代码的可读性和可维护性至关重要。不要羞于在代码评审中要求重命名，即使这占用了代码评审的大部分时间，甚至需要增加额外评审（在第二部分我们将讨论命名注释与最佳惯例）。许多情况下，有了 IDE 的帮助，重命名可以立即完成。这种情况，建议由开发人员处理。如果重命名需要更多时间（决定合适的名字或重构 IDE 无法达到的其他部分代码），可将其记录为注释，让程序员稍后处理。
- 文档化——像命名一样，在意图不明之处添加注释，或改进现有注释，这对可读性和可维护性至关重要。务必将其纳入代码评审范围。同样，对于简短注释，允许开发人员在代码评审期间临时添加，否则就将其作为一项任务来记录。请注意，可能存在遗忘留待日后处理注释的可能，因此添加此时尚且适当的注释会是将来代码合并工作的障碍。
- 不符合编码准则和惯例——编码准则和惯例可让我们维护大型代码库，并理解他人编写的代码。代码评审可防止非标准或不合规的代码进入自己的 repo。
- 缺陷、程序错误与潜在程序错误——不符合要求、缺少错误处理或未处理特定边缘案例、逻辑错误、错误设计、效率低下和错误假设——这些是代码评审的基础（在本博文的第二部分，我们将对其进行深入探讨，包括具体 C++ 项目）。
- 需求调查——代码评审期间，有些问题不会得到明确答案（“该操作是否由两个线程同时调用？” “如果我们到达这个阶段，规范是否允许取消该操作？”）。由此产生的任务将是调查本身，在许多情况下，对每个可能的答案所采取的行动也会记录于任务中。
- 值得一试——在某些情况下，可能会有不希望在本轮迭代程序中实施的建议，但又希望将其作为优化建议进行记录。这可能包括改进效率和简化代码的建议，这些建议不做强制要求，或者可能极其耗费时间。还要记住提出建议的两个原因：(a) 如果以后有时间或需要，可能会想实施这些建议；(b) 试图避免在未来评审中再次提出这些建议（所以有必要先行记录）。这样的注释可作为待办 (TODO) 注释编入代码，并附有到期日（到期后如不执行该待办，则予以删除）。
- 下次或许要换个方式——这类注释属于教育层面的评审。在某些情况下，有一段代码可用不同的方式编写，但目前不值得变更（可以运行，那就好，不要动它）。但是，可能还是要提醒程序员，这段代码可以用更优雅或更简单的方式来编写。即使在这种情况下并非针对某条待办注释，如果有时间，也可在评审中提出。
无意义注释——避免类似注释：当替代方法毫无优势——“方法不错，但可以用不同的方法”。确保避免不必要注释，这些注释在评审期间将会浪费时间，甚至转移对真正问题的关注。有些评审人员喜欢借用代码评审来标榜过去处理类似问题的英雄事迹。别这样。直接谈论评审中的代码，如果没有什么实质性贡献，也没必要非得分享自己的精彩事迹。
从何处着手评审？
评审代码变更
当评审小型新特性或程序错误修复情况时，评审重点通常是变更的代码。这种情况最好进行并行代码评审，比较新老代码。从检出文件清单开始评审，并从高层次解释每个文件需要变更的需求。随后按照适当的逻辑顺序，深入研究每个文件中的变更细节。如果从高层次解释开始，可从任何方向进行评审，自上而下或自下而上。
评审新代码
首先解释所要解决的需求和高层次设计，然后列出要添加的新类和文件。确保深入研究更复杂的部分（新算法、并发问题等）。以适当的逻辑顺序评审代码（如果与会者没有相应背景知识理解该话题，不要直接跳到新算法）。留出足够的时间来讨论代码中比较复杂的部分（如果只剩下 5 分钟来评审复杂算法，就再安排一次评审会议，不要试图在本不充裕的评审时间里讨论复杂事物）。再次强调，从高层次的解释开始，应该允许以任何方向执行评审，自上而下或自下而上。即便如此，对于新代码，大多数情况下，在得到高层次解释后，按照自上而下的方法会更为容易和有效。
当然，评审可能混含新代码和代码变更，在这种情况下，最好选择具有普适性的方法。在这种情况下，有关着手之处的逻辑，即是从新代码开始还是从代码变更开始，这取决于哪种顺序更易遵循和理解。
评审测试
通过单元测试——无论是新的单元测试还是对现有单元测试的更新——都应该是代码评审的一部分。确保新代码得到良好测试、测试覆盖真实场景，而非与需求无关的虚构场景，这由你自行把握。可以考虑从测试开始评审，因为在某些情况下，这是了解正在实现的特性或修复情况的最佳切入点。
总结
本部分涵盖了有效代码评审的基础知识。最后，代码评审并非另外的步骤。最好用它来保障代码质量、减少程序错误。执行有意义的代码评审有助于在过程早期突出问题，这时编码人员对于代码的印象尚且深刻，比起过程后期更容易修复问题，也能避免后续浪费他人的时间。
