C++代码审查终极指南

英文原文：https://www.incredibuild.com/blog/the-ultimate-guide-to-cpp-code-review-part-2

在第一部分中，我们花了一些时间讨论代码审查的广泛方面。本部分将专注于 C++，提供一个代码审查清单并提供一些最佳实践。你可以按任意顺序阅读它们，但我们建议回过头去阅读我们的之前的文章。

C++ 审查清单

代码审查清单永远不会全面，需要检查的问题、项目和潜在事项几乎是无穷无尽的。因此，制作一个涵盖所有潜在情况的清单是不可能的，而且执行起来更加困难。相反，让我们关注在 C++ 代码审查期间应该涵盖的广泛方面。

类别 1 - 需求和底层领域的理解

Joel Spolsky（Stackoverflow 的联合创始人，Fog Creek 的创始人，现在是 Glitch 的创始人，以及Joel-on-Software 博客的作者）曾经讲过一个关于他与微软联合创始人比尔·盖茨的审查的故事。

当时的 CEO 花了相当多的时间提问，包括一些看似随意的问题和更难的问题。问题越来越难，越来越尖锐，直到他问了一个“致命”的问题。当 Spolsky 能够回答这个问题时，这次审查得到了盖茨的通过。原因？比尔·盖茨想确保被审查的人掌控着材料，他通过在审查领域提出越来越难的问题来实现这一目的。

这是审查的一个重要部分。作为审查者，你应该理解你正在审查的功能或修复的问题，并能就实现细节提出好问题。这通常不是代码审查的主要关注点（即，通常不应占审查的大部分时间），但它应该是其中的一部分。

类别 2 - 非功能需求

非功能需求包括系统的那些部分，它们不直接服务于产品需求，但是允许产品正常工作（并符合法律法规），并且让我们在出现问题时分析问题。以下是一些要检查的关键事项：

可追踪性

• 代码是否具有适当的日志记录？
• 日志中是否包含了相关的可用信息？（在

出现错误时，我们需要的信息是什么？）

• 日志级别是否正确？（不要在正常情况下记录错误日志，也不要在错误情况下记录详细信息级别的日志）

安全性

• 代码中是否存在潜在的安全漏洞？
• 数据是否受到适当的保护？
• 是否遵循了最佳安全实践？

性能

• 代码是否遵循了性能最佳实践？
• 是否存在可能导致性能问题的瓶颈？
• 是否在关键部分进行了适当的优化？

类别 3 - 可维护性

可读性

C++ 是一门复杂的语言，所以代码审查者应该考虑如何使其尽可能易于阅读。以下是一些可以让代码更容易理解的方法：

• 保持函数简短且单一用途。然后确保它们的名称与它们实际执行的操作相对应（我们稍后会讨论名称）。
• 保持代码行简短。如果需要，将它们拆分成多行或使用辅助函数。
• 保持类简洁，遵循单一职责原则。
• 避免深层嵌套 - 如果你看到四层嵌套，你可能需要重构你的代码。如有必要，请使用辅助函数。
• 不要过于聪明。如果你的代码可能使其他人甚至你未来的自己感到困惑，请简化它。避免这样的代码：

a = b += c;

命名

在第一部分中，我们讨论了命名有多困难（以及为什么如此重要）。我们还讨论了为什么会这样。即便如此，恰当的命名使你的代码更具可读性和可维护性。

在开始为函数命名时，请遵循以下准则：

• 以清楚表明它们功能的方式命名函数。
• 以“get”开头的函数应该是 const，返回一个值，并尽可能不产生副作用。
• 以“set”开头的函数应该获取一个值并将其设置为内部变量。它们可能会或可能不会返回一个成功指示。
• 以“is”开头的函数应该是 const，返回一个 bool，并且通常不应该有副作用。
• 避免不必要的长名称。
• 避免过短的名称，无法提供足够的信息。
• 避免为不明确的操作重载运算符（例如，对于两个 Point 对象，operator*应该执行什么操作？不要使用它 - 写一个具有清晰名称的函数）。
• 对于非循环计数器的变量，请避免使用名称“i”。
• 避免为任何变量使用名称“temp”。

编码规范和约定

• 将类保留在相应的 cpp 和 h 文件中；不要将多个类放在同一个文件中（嵌套类除外）。
• 选择 east-const 或 west-const，不要混合使用。
• 使用 auto 声明变量。但是在函数返回类型和函数参数上无故避免使用 auto（C++20 函数参数可以声明为 auto，但函数变为模板函数）。
• 不要使用#define 声明常量。
• 除非没有其他方法可以实现所需的结果，否则不要使用#define 声明宏，在这种情况下，代码应该是您的基础结构代码的一部分。
• 使用 constexpr 或 constinit 作为常量并使其成为 static。
• 不要在全局范围内声明 enum。可以使用 enum class 或根据上下文将 enum 放在适当的类中。
• 避免为整个命名空间使用 using namespace 以避免名称污染。
• 确保拥有一个内部广泛接受的自己的编码规范和约定清单。

简单代码

• 代码应该尽可能简单，但不能比这更简单（参见爱因斯坦关于简单的观点）。
• 尽可能遵循零规则。通常不需要手动分配内存，将这些分配保留给应位于项目基础结构部分的特殊基础结构类。（如果您必须偏离零规则：我们将在下面讨论三规则和五规则）。
• 使用辅助函数。是的，我们知道我们已经在上面说过了。
• 避免类型之间的循环依赖，使用接口而不是实际类型。
• 避免不必要的抽象。如果您有一个抽象，它应该有一个明确的原因（对未知的未来准备并不是一个好理由）。
• 推迟你的继承。将其用于小事物，如状态和策略，而不是用于需要管理状态或策略的实际对象这样的大事物。开发者和技术撰稿人都应该只是员工，或者使用一个角色字段来表示。

将数据保持在适当的上下文、私有和常量中

• 不要使用全局变量。坚决不要。
• 默认情况下，您的所有数据成员都应该是私有的。您应该有一个很好的理由拥有受保护的数据成员，没有理由拥有公共数据成员。
• 您的静态成员也应该是私有的。
• 力求保持事物为 const。确保参数以 const 引用传递，函数被声明为 const，除非需要修改。
• 成员函数通过引用或指针返回数据成员，没有 const，通常是邪恶的。尽量避免它们。如果您确实需要通过引用返回一个成员，请基于接口而不是实际类型公开它。
• 确保成员的逻辑常量性。编译器确保物理常量性。保护您的逻辑常量性是您的责任。
• 如果用户无法从函数的名称和签名中判断该函数是否可能更改调用对象的成员，那么函数名称和签名都有问题。

避免代码重复

• 代码重复是不好的，它使代码膨胀，为未来的维护增加了工作量。而且容易出现错误，比如在一个地方修复或更改逻辑，而在其他地方意外保留旧逻辑。此外，代码重复容易导致复制粘贴错误，粘贴的代码需要更新以适应新的用途，但程序员忘记更新它或者只更新了部分。
• 即使代码看起来不完全相同，也不要重复代码，尝试将事物移到基类或函数中。这是值得努力的。
• 模板是一个很好的工具，不仅对于专家或基础设施团队，使用模板来避免代码重复。

适当的文档

不要太多，也不要太少。如果你想拥有好的注释，请阅读 Stackoverflow 联合创始人 Jeff Atwood 的文章，没有注释的编程 和 代码告诉你如何做，注释告诉你为什么。

记录“为什么”应该包括：

• 为什么这个循环必须在下一个循环之前，即使看起来很奇怪。
• 为什么我们没有在这里做明显的事情（实际上并不起作用，我们试过了……）。

确保你有这类注释，它们会节省你的时间，让你免受恶心的 bug。

类别 4 - 可用性和正确性

这是指具有正确代码、无错误且性能良好的工作系统。这应该包括以下内容：

不惜一切代价避免未定义行为，力求避免未指定行为

在 C++ 中，您永远不应该使用 “但它起作用了” 这个论点。依赖未定义行为的任何代码都是有问题的；依赖未指定行为的代码在重新编译时（使用不同的编译标志，或者使用另一个编译器重新编译）可能会改变其行为。

确保避免以下情况：

1. 有符号整数溢出
2. 不正确的类型转换
3. 访问已释放但偶然仍可访问的内存
4. 忘记在多态层次结构中实现虚析构函数并动态分配派生类对象。
5. 其他未定义的行为（cppcon 的热门话题）。

即使你必须重写某个你非常依赖的代码片段，也要避免这些问题。

资源管理

在 C++ 中，资源管理应被视为一个已解决的问题。RAII 是神奇的词汇，智能指针是其中之一的工具。只需使用它们。当然，使用标准库容器（或其他托管容器）。

• 尽可能使用零规则保护自己。使用智能指针和适当的容器。
• 如果由于某种原因您需要实现一个析构函数，请记住三法则：实现或删除复制构造函数和复制赋值运算符。（如果你只声明一个默认的虚析构函数，不要阻止复制和赋值。相反，为所有四个操作声明默认值：复制和移动构造函数以及赋值）。
• 确保所有指针都初始化为 nullptr 或有效地址。
• 对于唯一所有权，请使用 std::unique_ptr，且不要通过引用传递 unique_ptr。
• 对于共享所有权，请使用 std::shared_ptr，尽量不要通过引用传递。
• 避免在用户代码中使用 new 和 delete，使用 std::make_unique 和 std::make_shared 分配内存。
• 检查对 unique_ptr::get 和 shared_ptr::get 的调用是否误用了指针（例如，不要将其意外地传递给另一个智能指针）。
• 检查对 unique_ptr::release 的调用是否获取了返回的指针，并将其传递给另一个智能指针或释放它。
• 对于任何需要释放的资源，请使用锁保护和 RAII 技巧。

悬空指针和引用，无效引用和迭代器

C++ 的一种危险是使用悬空或无效的引用、指针或迭代器。确保：

• 在调用 push_back 之前，不要使用指向向量的迭代器（确保遵循失效规则，另请参阅此处）。
• 不要使用对临时对象的引用（如Facebook 奇怪的循环 C++ Bug中所示）。

转换、类型和类型安全

转换是棘手的，特别是在绕过编译器和不使用强类型时。错误的类型和错误的转换已知曾导致卫星坠毁，所以你最好小心：

• 使用 C++ 强制转换运算符（static_cast、reinterpret_cast、const_cast 和 dynamic_cast），而不是 C 风格的强制转换。它们说明了你想执行的操作。
• 不要使用 const_cast 移除 const 属性并更改变量，这是未定义的行为。
• 为了类型安全，请使用强类型（参见 Joe Boccara 的 NamedType 库）。

错误处理

确保代码不会忽略错误。

• 如果有一个 if 没有 else，问一下 - else 中应该发生什么？并不是说你总是需要一个 else，但提问可能会暴露出你之前没有意识到的需求。
• 同上，覆盖 switch 中包括默认值在内的所有情况（即上面都不符合）。
• 如果有可能出现错误，假设它会出现。如果你不立即处理错误情况，这是可以的。只需在那里留下一个 TODO 注释和错误日志，或者如果你正在使用异常，则抛出一个异常。
• 让异常传播出去，中止程序，比吞没异常要好。不要吞没异常！
• 如果我们有一个递归算法，我们是否受到栈溢出情况的保护？算法可能达到的最大深度是多少？考虑处理这种情况。
• 一般来说，优先考虑安全失败而不是崩溃，但优先考虑崩溃而不是处于错误状态下运行。

并发性

如果您的程序使用多个线程运行，则代码审查中必须提出“是否线程安全”的问题。具体来说，您应该问：

• 从多个线程访问的数据是否在所有线程中使用相同的锁锁定，或者使用适当的原子操作进行处理？
• 阻塞操作最好使用超时来调用，至少在超时发生时进行日志记录，以便更好地追踪并避免死锁。
• 考虑使用 RCU（读、复制、更新）来避免长时间锁定，或者完全避免锁定。
• 除非你以此为生，否则不要发明你自己的无锁算法。陷入ABA 问题或类似问题太容易了。使用现有的算法和数据结构。
• 在使用标准库容器时，请确保遵循它们的线程安全规则，另请参阅此处。
• 尝试为您的关键多线程场景模拟竞态条件进行单元测试。

性能

我们使用 C++ 编写的原因之一是性能 —— 达到更低的延迟和更高的吞吐量。

• 记住 vector 是最好的。如果您从其他选择中选择，请记录原因！
• 在大多数情况下，使用标准库算法比实现自己的版本更好（并非仅仅出于性能原因）。
• 不要在尚未知道瓶颈的事情上投入几天的优化。但如果多花一个小时，更愿意投入更多时间和思考来编写更高效的代码。
• 考虑算法复杂性，优先选择 O(n) 算法而不是 O(n^2) 算法，这不是预优化。
• 确保通过按值获取对象或在获取 const ref 时隐式转换为临时对象时，不要创建冗余副本，如果您发送的类型不完全匹配。
• 确保在相关情况下跳出循环。是的，每个人都知道这一点，但有时我们还是会忘记。
• 如果看到一个三层嵌套循环，请询问一些关于那里发生了什么的问题，尝试稍微调查一下该算法。它是否有效？
• 记住，循环内的简单调用可能包含一个循环。
• 聪明地使用移动语义：
• 再次强调，遵循零规则是您的朋友，但如果您没有遵循零规则，请记住五规则，确保实现或声明默认的移动操作。
• 如果实现移动操作，请记住将它们标记为 noexcept。
• 记住在需要时使用 std::move 和 std::forward（但不要在仍在使用的变量上使用它们，或在返回本地变量时使用它们）。

最佳代码审查实践

你可以拥有世界上最好的检查清单，但如果你没有遵循良好的代码审查实践，它将没有太大的用处。在这篇博客的第一部分中，我们讨论了代码审查的一些更技术性的实践，但是让我们为代码审查本身添加一些重要的条目：

1. 分段审查 — 并给自己足够的时间来完成。大多数人在一次会议中无法理解超过 12 个类或超过 600 行代码。一些开发人员发现在 150 行后，他们的注意力开始分散。找出适合您和您的团队的方法，然后设定自己的标准。
2. 每次审查时间不超过 90 分钟。之后，休息一下，清空头脑，然后回来再审查代码。长时间盯着相同的代码，你会开始忽略一些问题。
3. 在审查前构建和测试。我们已经在第一部分中讨论过这个问题，但它足够重要，值得重复。如果你要求开发人员检查你的代码，请确保通过首先构建和测试你的代码，让他们觉得值得。代码审查不是用来发现自动测试可以发现的 bug 的工具。理想情况下，你应该带着静态代码分析的结果，以及你可能忽略的任何建议的评论来进行代码审查。
4. 确保在代码合并之前始终进行代码审查。代码审查是一个门槛！如果代码没有通过审查，不要打开这个门。
5. 使用代码审查来教育和规范团队对'好'代码的理解。每个人对代码质量的标准都不同，因此确保你们在进行代码审查时都在同一页上。确保每个人都理解对任何建议更改的原因，使建议具有合理性、具体性和生产力。这样，你就可以避免在未来的代码审查中对类似错误进行评论。
6. 谨慎选择评论并给予合理的优先级。再次强调，这已经在第一部分讨论过，仍然足够重要以至于需要重复：并非所有评论都具有相同的优先级，请确保优先考虑你的评论，并明确哪些评论需要在代码合并之前进行修复，哪些可以推迟到以后的版本。
7. 使评论礼貌、深思熟虑、有道理且直接。你们是一个团队，每个人都在尽力编写优秀的代码。尝试在保持尊重与直接明确问题之间找到平衡。Google 的评论指南总是一个很好的参考起点。

想要构建真正有效的代码吗？让 C++ 代码审查成为你的日常工作

代码审查对于任何编程语言来说都不是一个可选的附加功能，但对于 C++ 来说确实是必须的。良好的 C++ 代码审查对于确保代码清晰、逻辑、高效和正确至关重要。让其他人检查你的代码确实是唯一能找到那些让你的代码从“在我的机器上工作”变为“准备生产”的小调整的方法。

额外阅读

• C++ 核心准则
• Rob Pike（Go 语言的创建者之一）关于 C 编程的建议集（是的，C，但它在很大程度上也适用于 C++）。特别关注有关注释和过程名称的部分，但整个内容都是真正的珍珠。
• 编写易读代码的 13 个原则
• 一个简化的 C++ 代码审查清单
• 另一个 代码审查检查清单