C++ 代码评审最终指南——第 2 部分

Amir Kirsh
Amir Kirsh / 5月 18 2022
C++ 代码评审最终指南——第 2 部分

在第 1 部分中,我们对代码评审做了一个广泛讨论。这一部分将重点讨论 C++,提供一个代码评审清单和一些最佳实践。您可以按任何顺序阅读,但是我们建议您先返回去阅读一下我们之前的帖子

C++ 评审最终清单

代码评审清单从来都不是全面的——因为要检查的问题、项目和潜在事情的数量几乎是无穷无尽的。因此,制定一份涵盖所有潜在可能性的清单是不可能的,也更难执行。相反,我们将重点关注在 C++ 代码评审期间应该涵盖的更为广泛的方面。

类别 1——对底层域的要求和理解

Joel Spolsky(Stackoverflow 的联合创始人,Fog Creek(现为 Glitch)创始人,Joel-on-Software 博客的作者)曾经讲述过他与微软联合创始人比尔·盖茨做的一次评审

时任首席执行官花了很长时间问了一些看似随机的问题,以及一些更难的问题。问题变得越来越难,越来越尖锐,直到他问了一个“杀手”问题。Spolsky 一回答,盖茨就通过了评审。但这样做的原因是什么?比尔·盖茨希望确保受评人员控制材料,他通过在评审领域提出越来越难的问题来做到这一点。

这是评审的重要一部分。作为一名评审人员,您应了解正在评审的特性或错误修复情况,并能够就实现细节提出好的问题。这通常不是代码评审的主要重点(即,通常不应占用评审的大量时间),但它应该是评审的一部分。

类别 2——非功能性需求

非功能性需求包括系统中不直接服务于产品需求但允许产品正常工作(并符合法律法规)的部分,在事情进行的不顺利时,我们可以分析问题。以下是一些需要注意的关键事项:

可追踪性

  • 代码是否有适当的日志记录?
  • 这些日志中是否包括相关可用信息?(最令人沮丧的经历之一是调试生产问题,有一个日志行可以提供帮助,但该日志行中缺少所需的信息)。
  • 是否有任何其他可追踪性要求?(例如,通过 SNMP 上报给 NMS)。

隐私和安全

  • 确保代码不会记录或打印任何敏感信息。
  • 代码中不应含有任何硬编码密码或其他敏感数据。
  • 切勿假设任何外部输入是有效的,避免注入威胁。

(阅读我们有关 C++ 漏洞的博客文章,了解有关 C++ 安全的更多信息)。

类别 3——可维护性

可读性

C++ 是一种复杂的语言,因此代码评审人员应考虑使用不同的方法尽量使其具有可读性。以下是一些可以让代码更容易理解的方法:

  • 保持功能简短且用途单一。确保代码名称与其实际用途相对应(我们将在之后对代码名称进行讨论)。
  • 保持代码行简短。如果必要,将代码分为几行或使用辅助函数。
  • 保持代码类别简洁并遵循单一责任原则。
  • 避免深度嵌套——如果您看到四层嵌套,您可能应该重构代码。如果需要,也可以使用辅助函数。
  • 不要自作聪明。如果您的代码让他人甚至是未来的你自己感到困惑,则应简化代码。避免这样的代码:
a = b += c;

命名

在第 1 部分中,我们讨论了命名的困难程度(以及为什么命名如此重要)。我们还讨论了为什么会出现这种情况。尽管这样,但适当的命名会使代码更具可读性和可维护性。

开始命名函数时,请遵循以下准则:

  • 用一种明确指示函数功能的方式命名函数。
  • 以 ‘get’ 开头的函数应为常量,返回一个值,并且尽可能没有副作用。
  • 以 ‘set’ 开头的函数应获取一个值并将其设置为内部变量。它们可能会或可能不会返回成功指标。
  • 以 ‘is’ 开头的函数应为常量,返回一个布尔值,而且通常不应有副作用。
  • 避免使用不必要的长名称。
  • 避免使用不能传达足够信息的简短名称。
  • 避免对不清楚的操作重载操作符(例如,操作符*应该为两个 Point 对象做什么?切勿使用它——应重写一个具有明确名称的函数)。
  • 对于不是循环计数器的变量,避免使用名称 ‘i’。
  • 避免对任何变量用 `temp` 命名。

编码准则和惯例

  • 将您的类保存在相应的 cpp 和 h 文件中;切勿将多个类放置在同一个文件中(嵌套类除外)。
  • 选择 east-const 或 west-const——不要混搭。
  • 使用 `auto` 声明变量,但是避免在函数返回类型和函数参数上无故使用 `auto`(C++20 函数参数可被声明为 `auto`,但是该函数随后会成为模板函数)。
  • 切勿使用 #define 声明常量。
  • 切勿使用 #define 声明宏,除非没有其他方法可以实现所需的结果,在这种情况下,代码应该是基础架构代码的一部分。
  • 对常量使用 `constexpr` 或 `constinit`,并将其设置为‘静态’。
  • 切勿在全局范围中声明 `enum`。可以使用 `enum class`,也可以根据上下文将 `enum` 放在适当的类中。
  • 避免对整个命名空间使用 `using namespace`,以避免名称污染。
  • 确保有一个内部广泛接受的编码准则和惯例列表。

简码

  • 代码应尽量简单,但是不能太简单
  • 尽可能使用零规则。通常不需要用户手动分配内存,只需将那些用于特殊基础架构类的内存保留在项目的基础架构部分即可。(如果不得不偏离零原则:我们将在下文讨论三规则和五规则)。
  • 使用辅助函数。是的,我们已在上文中提到过。
  • 避免类型之间的循环依赖,使用接口而不是实际类型。
  • 避免不必要的抽象概念。如果您有一个抽象概念,则应有明确的理由(对一些未知的将来做好准备并不是一个很好的理由)。
  • 推迟继承。将其用于状态和策略等小事,而不是用于需要管理状态或策略的实际对象等大事开发人员和技术文档工程师都应仅仅是一名雇员,也许有一个角色领域。

将数据保存在适当的上下文中,使其为私有并为常量

  • 切勿使用全局变量。绝对不行。
  • 所有数据成员都应默认为私有。您应该有充分的理由拥有受保护的数据成员,而没有理由拥有公共数据成员。
  • 您的静态成员也应为私有。
  • 目的是保持为常量。确保参数作为常量引用移动,除非需要修改,否则函数声明为常量。
  • 通过引用或指针返回数据成员的成员函数(不带常量)通常有害无益。应尽量避免使用这类函数。如果您确实需要通过引用返回数据成员,则应根据接口而不是实际类型公开该成员。
  • 确保成员的逻辑常量性。编译器确保物理常量性。您负责保护逻辑常量性。
  • 如果用户无法区分函数名称和签名判断函数是否可能更改调用对象成员,则函数名称和签名有问题。

避免代码重复

  • 代码重复很不好,它会使代码膨胀,使将来的维护工作量更大。而且代码重复很容易导致错误出现,比如在一个位置修复或更改逻辑,而不小心将旧的逻辑保留在其他地方。此外,代码重复很容易出现复制粘贴错误,导致粘贴的代码必须更新以适应其新用途,但程序员忘记更新或仅部分更新。
  • 因此,即使看起来不完全相同,也不要重复代码——尝试将内容移至基类或函数中。这么做会物有所值。
  • 模板是一个非常好的工具,不管是专家还是基础架构团队,都可以使用模板来避免代码重复。

适当的文档记录

不要太多,也不要太少。如果您想要对注释有个较好的理解,请阅读 Stackoverflow 联合创始人 Jeff Atwood 的帖子无注释编码代码说方式,注释说原因

记录“原因”包括:

  • 为什么这个循环必须在下个循环之前,尽管它看起来很奇怪。
  • 为什么我们没有在这里做显而易见的事情(那真的不起作用,我们尝试了…)

确保您有这些类型的注释——它们将为您节省时间,并帮您避免那些令人讨厌的错误。

类别 4——可用性和正确性

这指的是一个代码正确、性能良好且没有错误的工作系统。其中应包括以下内容:

不惜一切代价避免未定义的行为,并避免未指定的行为

切勿在 C++ 中使用“但它有效”参数。任何依赖未定义行为的代码都会被破坏;任何依赖未指定行为的代码在重新编译时都可以改变其行为(使用不同的编译标志,或者当使用另一个编译器重新编译时) 。

确保避免:

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

一定要避免上述行为,即使必须重写一段你非常依赖的代码。

资源管理

在 C++ 中,资源管理应该被视为一个已解决的问题。RAII 是一个神奇的词,而智能指针是其中一个工具。可以利用它们来进行资源管理。当然,还得使用标准库容器(或者其他托管容器)。

  • 尽可能使用零规则,以确保安全。使用智能指针和适当的容器。
  • 如果出于某种原因需要实现析构函数,请记住三规则:实现或删除复制构造函数和复制赋值运算符。(如果只声明了默认的虚拟析构函数,请不要阻止复制和赋值。相反,应声明所有四个的默认值:复制和移动构造函数和赋值)。
  • 确保所有指针都已初始化为 nullptr 或一个有效地址。
  • 使用 std::unique_ptr 表示唯一所有权,切勿通过参考传递 unique_ptr。
  • 使用 std::shared_ptr 表示共享所有权,最好不要通过参考传递。
  • 避免在用户代码中使用新建和删除,使用 std::make_unique 和 std::make_shared 分配内存。
  • 检查对 unique_ptr::get 和 shared_ptr::get 的调用是否滥用指针(例如,不要意外地将它传递给另一个智能指针)。
  • 检查对 unique_ptr::release 的调用是否接受返回的指针并将其传递给另一个智能指针或释放它。
  • 对任何应该释放的资源使用锁守卫和 RAII 技术。

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

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

转换、类型和类型安全

转换很棘手,尤其是在绕过编译器和不使用强类型时。众所周知,错误的类型和错误的转换会致卫星坠毁,因此您最好小心:

  • 使用 C++ 类型转换操作符(static_cast、reinterpret_cast、const_cast 和 dynamic_cast),而不是 C 风格类型转换。他们会按照您的要求表述内容。
  • 切勿使用 const_cast 删除常量并更改变量,因为这是未定义的行为。
  • 使用强类型可实现类型安全(见 Joe Boccara 的 NamedType )。

错误处理

确保代码不会忽略错误。

  • 如果有一个 `if` 没有 `else`,则应提问——在 `else` 中应发生什么?这并不是说您总是应该需要一个 ‘else’,而提问可能会揭示您没有意识到的需求。
  • 如上所述,应在 switch 中涵盖所有情况,包括默认情况(即,上述情况均不适用)。
  • 如果某件事可能会失败,则假设它会失败。如果不立即处理错误案例,也可以。如果您正在处理异常,只需在此处留下 TODO 注释和错误日志或引发异常。
  • 让异常传播出去,中止程序,比抑制异常要好。切勿抑制异常!
  • 如果使用递归算法,那么我们是否可以避免堆栈溢出场景?该算法的可能达到的最大深度是多少?考虑处理这种情况。
  • 通常,故障安全优于崩溃,但是崩溃又优于在不良状态下运行。

并发性

如果您的程序运行多个线程,那么在您的代码评审中必须提出“线程是否安全”这一问题。尤其应提出以下问题:

  • 从多个线程访问的数据是否在所有线程中都用相同的锁锁定,或者用适当的原子操作处理?
  • 最好使用超时调用阻断操作,至少在超时发生时进行记录,以便更好地跟踪和避免死锁。
  • 可以考虑使用 RCU(读、复制、更新)操作来避免长时间锁定或完全锁定。
  • 切勿发明任何无锁算法,除非这是您的工作。否则很容易陷入 ABA 问题或者类似问题。可使用现有算法和数据结构。
  • 当使用标准库容器时,请确保遵循其线程安全规则,另见此处
  • 尝试进行单元测试,模拟关键多线程场景的竞争条件。

性能

我们用 C++ 编写代码的原因之一是性能——为了实现更低的延迟和更高的吞吐量。

  • 切记,向量是最好的。如果做出其他选择,请记录原因!
  • 大部分情况下,使用标准库算法比用自己的版本要好(这不仅仅是出于性能原因)。
  • 不要花时间对未知瓶颈进行预优化。但是,如果要多花一个小时,最好多花一点时间和精力来编写更高效的代码。
  • 考虑到算法的复杂性,首选 O(n) 算法而不是 O(n^2) 并不是预优化。
  • 如果发送的类型不完全匹配,请确保不要通过按值获取对象或在获取常量引用时隐式转换为临时对象来创建冗余副本。
  • 确保在相关情况下打破循环。是的,每个人都知道这一点,但有时我们会忘记。
  • 如果您看到一个三层嵌套循环,请询问一些关于这是什么情况的问题,尝试研究一下算法。这有效吗?
  • 记住,循环内的简单调用可能会含有一个循环。
  • 巧妙地使用移动语义:
    • 切记,零规则对您有益无害,但是如果你无法使用零规则,可使用五规则并确保实现或声明默认移动操作。
    • 如果您实现移动操作,切记将其标记为 `noexcept`。
    • 记住,必要时应使用 std::move 和 std::forward(但是切勿将其用于仍在使用的变量或在返回局部变量时使用)。

代码评审最佳实践

您可以拥有世界上最好的清单,但如果您不遵循良好的代码评审实践,那么它就没有多大用处。在这篇博客的第 1 部分中,我们讨论了一些有关代码评审的技术实践,现在我们来聊聊就代码评审本身而言很重要的一些事项:

  1. 分小块评审——给自己足够的时间来做这件事。大多数人无法在一次会议上掌握超过 12 个类或 600 行代码。有些开发人员发现,他们的注意力在 150 行之后就会分散。所以,应找出适合您和您的团队的方法,并制定自己的标准。
  2. 一次评审不要超过 90 分钟。之后,应休息片刻,清理思绪,然后再回来继续评审代码。如果长时间盯着同一个代码,您会错过一些东西。
  3. 评审前先构建和测试。在这篇博客的第 1 部分中我们已经讨论过着这个问题,但是它的重要性足以让我们再重复讲述一遍。如果您想让开发人员检查您的代码,请先构建和测试您的代码,以确保它值得开发人员花时间检查。代码评审并不是检测错误的工具,自动化测试才是。理想情况下,您还应该带着静态代码分析结果以及对可能忽略的任何建议的评论来进行代码评审。
  4. 确保始终在代码合并前进行代码评审。代码评审是一道大门!如果代码未通过评审,切勿打开这扇大门。
  5. 通过代码评审教育和标准化您的团队对什么是代码的理解。每个人都有不同的代码质量标准,因此请确保所有人都按照同一标准进行代码评审。确保每个人都理解任何建议修改的原因,使建议合理、具体且富有成效。这样您可以避免在将来的代码评审中对类似错误进行注释。
  6. 仔细选择注释并给出合理的优先级。需再次说明的是,虽然我们已经在第 1 部分中做过讨论,但是仍值得在此重复说明:并不是所有注释都有同样的优先级,所以请确保对您的注释进行优先级划分,并明确哪些注释需要在代码合并之前进行修复,哪些注释可以推迟到以后的版本中。
  7. 注释应礼貌、周到、合理和直接。您是一个团队,每个人都在尽最大努力编写好的代码。试着在尊重和直接、明确地说出任何问题之间找到平衡。Google 的注释指南就是一个很好的起点。

 

想要构建真正有用的代码?使 C++ 代码评审成为日常工作的一部分

不论对任何语言,代码评审都不是可选的附加组件,但对 C++ 而言却是必须的。如果您想确保代码干净、有逻辑、高效、而且最重要的是正确,那么良好的 C++ 代码评审是必不可少的。实际上,额外关注代码是找到将其从“在我的机器上有效”转变为“准备投入生产”的小调整的唯一办法。

更多信息

订阅博客

阅读 Incredibuild 独家内容

Amir Kirsh

Amir Kirsh,Incredibuild 的开发推广师,在 Tel-Aviv-Yaffo 学院和 Tel-Aviv 大学任教,主讲 C++。 Amir 此前是一家创业公司的首席技术官和研发副总裁,后来这家创业公司成功被 Comverse 收购,Amir 则成为 Comverse 的首席程序员。此外他也是每年 Core C++ 论坛的组织者和 ISO C++ 以色列国家组织的成员。