【软件工程】从混乱到秩序：Code-Review代码审查助力代码质量飞跃

1. 概述

对于一个嵌入式项目来说，代码质量往往决定了项目的成败。然而，随着项目规模的扩大和团队成员的增多，代码质量的问题也将日益凸显。混乱的代码结构、难以理解的逻辑、潜在的安全隐患……这些问题都可能成为项目推进和质量保障的绊脚石。而Code-Review，作为一种重要的代码质量控制手段，正逐渐被越来越多团队所重视。

Code-Review，顾名思义，就是对代码进行审查的过程。它不仅仅是简单地检查代码是否存在语法错误或逻辑问题，更是一种团队协作和知识共享的方式。通过Code-Review，团队成员可以相互学习、相互借鉴，共同提升代码质量。

2. Code-Review审查项

那么，Code-Review究竟如何助力代码质量飞跃呢？接下来，我将基于我十几年工作生涯中的Code-Review经验，为大家一一揭秘，希望能帮助大家在以后的代码开发过程中，逐步提升代码质量，促进项目顺利上线。

2.1 编码格式

审查方法

首先，公司内部所有开发人员协商确定好一份大家都认可的编码风格，形成一份开发人员的《代码开发规范SOP》。内容可以包含编码风格及其一些其它共识性的规则，比如：

1. if-else即使只有一行代码，也必须加大括号
2. 代码缩进使用4个空格而不是Tab
3. 所有变量定义时必须进行初始化
4. 废弃代码建议直接删除而不是注释掉
5. …
6. …
7. …

处理方法

不符合SOP的，在Code-Review平台评论审查意见，提交人跟踪意见，直到问题解决后，才能允许merge进代码仓库。

2.2 commit message检查

审查方法

代码审查者，要能真实轻松看明白本次commit的代码是在做什么：

1. “fix bug/error on helloword”这类无价值的通用性描述语句禁止合入，要求必须准确描述本次commit的意图。
2. message格式不合规的禁止合入，比如message过长要求提交人将其拆分为message title和message content，两者中间用空行间隔开。详细冗长的描述可以放在message content中。
3. message和patch内容有出入的禁止合入；
4. message缺少必要验证结果呈现的禁止合入；

处理方法

不符合的，在Code-Review平台评论审查意见，提交人跟踪意见，直到问题解决后，才能允许merge进代码仓库。

2.3 patch原子性检查

审查方法

1次提交是否只做了一件事。

处理方法

1次提交做了多件事的，要求拆开后分多次提交。

2.4 新增函数是否有函数头

审查方法

1. 本模块对外接口必须要函数头，static和inline的根据实际需要；
2. 函数头格式符合公司和团队要求，参考：

/**
 * @brief The handle any demux operation that could not be expressed by other functions.
 * @param[in] this    The reflexive to the vma_demux_plugin itself.
 * @param[in] private The points to plug-in private data.
 * @param[in] cmd     The command word to specific control operation.
 * @param[inout] arg  The argument related to cmd.
 * @return  CONTROL_TRUE, CONTROL_SUCCESS, CONTROL_FAILURE, CONTROL_FALSE, CONTROL_UNKNOWNCMD.
 */

3. 更新参数列表、函数名、返回值、核心逻辑时对应更新函数头描述；

处理方法

不符合的，在Code-Review平台评论审查意见，提交人跟踪意见，直到问题解决后，才能允许merge进代码仓库。

2.5 新增文件是否有文件头

审查方法

新增源文件必须写文件头，格式符合公司和团队要求，参考：

/**
 * Copyright (C) 2020 替换为公司名, All rights reserved.
 *
 * @file 替换为文件名
 * @brief 替换文文件的功能描述
 * @author 替换为文件创建作者的名字或者邮箱
 * @date 替换为文件创建日期
 */

注意：

1. Copyright的年份要写成当年；
2. 头文件必须做重复包含防御；
3. 重构时根据需要更新文件头；
4. 文件权限合理；

示例如下：

/**
 * Copyright (C) 2020 Alibaba.inc, All rights reserved.
 *
 * @file main.h
 * @brief The main head file for nshell.
 * @author this_is_a_demo@alibaba-inc.com
 * @date 2020/11/27
 */

处理方法

不符合的，在Code-Review平台评论审查意见，提交人跟踪意见，直到问题解决后，才能允许merge进代码仓库。

2.6 新增头文件和源文件匹配

审查方法

1. 原则上功能模块每个cpp/c文件要有一个对应的头文件，特殊情况可让步；
2. 头文件不能循环include；

处理方法

不符合的，在Code-Review平台评论审查意见，提交人跟踪意见，直到问题解决后，才能允许merge进代码仓库。

2.7 重名宏重复定义

审查方法

C文件的重名宏定义需要先undef；用于避免编译warning。

处理方法

不符合的，在Code-Review平台评论审查意见，提交人跟踪意见，直到问题解决后，才能允许merge进代码仓库。

2.8 看到if就去找else

审查方法

1. 条件路径封闭检查，是否存在不在条件全路径内的场景；
2. 条件分支太多建议改成switch-case语句，或者其他更清晰的实现；

处理方法

不符合的，在Code-Review平台评论审查意见，提交人跟踪意见，直到问题解决后，才能允许merge进代码仓库。

2.9 看到malloc就去找free

审查方法

1. 是否没有free；
2. 是否存在重复free路径；
3. free后指针是否置NULL；

处理方法

不符合的，在Code-Review平台评论审查意见，提交人跟踪意见，直到问题解决后，才能允许merge进代码仓库。

2.10 看到while/for就去找边界条件

审查方法

循环退出的边界处理是否妥当，存不存在死循环等异常路径；

处理方法

不符合的，在Code-Review平台评论审查意见，提交人跟踪意见，直到问题解决后，才能允许merge进代码仓库。

2.11 看到数组去找边界

审查方法

1. 数组起始边界一般都能注意到，结束边界容易疏忽；
2. 数组名退化为指针时注意用法，数组名不能做左值；

处理方法

不符合的，在Code-Review平台评论审查意见，提交人跟踪意见，直到问题解决后，才能允许merge进代码仓库。

2.12 看见内存拷贝关注长度

审查方法

1. 源和目的地址范围匹配；
2. 字符串处理函数使用带n版本，如strncpy；
3. 环形拷贝注意position，避免回卷覆盖引发错误；

处理方法

不符合的，在Code-Review平台评论审查意见，提交人跟踪意见，直到问题解决后，才能允许merge进代码仓库。

2.13 看到锁就去找解锁

审查方法

1. 锁上下文是否太大，只针对必要的上下文上锁；
2. 找找解锁，避免死锁；
3. 思考当前上下文中上锁是否有其他业务风险；

处理方法

不符合的，在Code-Review平台评论审查意见，提交人跟踪意见，直到问题解决后，才能允许merge进代码仓库。

2.14 看见return就去前面找资源

审查方法

返回前关注资源回收处理，如解锁、json对象释放等；

处理方法

不符合的，在Code-Review平台评论审查意见，提交人跟踪意见，直到问题解决后，才能允许merge进代码仓库。

2.15 变量初始值

审查方法

所有变量定义必须赋初值，避免可能出现的未初始化访问带来异常结果。

处理方法

不符合的，在Code-Review平台评论审查意见，提交人跟踪意见，直到问题解决后，才能允许merge进代码仓库。

2.16 栈内内存分配控制（一般用于RTOS系统）

审查方法

1. 不能在栈内开辟大空间，防止爆栈；
2. 超过1K的buffer不要放在函数栈内，比如int arr[2048];;

处理方法

不符合的，在Code-Review平台评论审查意见，提交人跟踪意见，直到问题解决后，才能允许merge进代码仓库。

2.17 函数体规模不宜太大

审查方法

1. 函数体非空非注释的有效行数不超过100行；（google给的是40，我觉得小了点）
2. 重复语句太多看看能不能去重；

处理方法

超20%以内可以斟酌，20%以上必须解决才能合入；

2.18 圈复杂度不能太高

审查方法

1. 函数圈复杂度不超过10；
2. 工具扫描最准，简单函数体粗略计算下就可以；

处理方法

拆解或者重写降低圈复杂度后重新提交审查

2.19 业务逻辑疑问和模块owner确认

审查方法

1. 方案实现细节是否可以改进；
2. 资源使用上能否调整；

处理方法

不符合的，在Code-Review平台评论审查意见，提交人跟踪意见，直到问题解决后，才能允许merge进代码仓库。

2.20 针对C++特性

审查方法

1. 内部通用的namespace建议封装成宏；
2. using namespace不能放在头文件中，要放在cpp源文#include下方；
3. 其他

处理方法

不符合的，在Code-Review平台评论审查意见，提交人跟踪意见，直到问题解决后，才能允许merge进代码仓库。

2.21 SOLID原则遵循

审查方法

1. 上层实现不依赖底层模块，应该依赖于抽象，尽量反转依赖；
2. 对扩展开放，对修改封闭，涉及到接口变更必须对齐用法现状；
3. 主要在架构设计审评时，一般代码review阶段较少涉及；

处理方法

2.22 想想哪里还能再优化

审查方法

最后六一个开放性审查项，没有具体要求，只是希望审查者在审查代码时深入思考一个问题：“哪里还可以更优化”。

处理方法

自行思考

3. 总结

要想充分发挥Code-Review的作用，我们还需要注意一些细节。首先，要保持开放和包容的心态。在审查他人代码时，我们要尊重他人的劳动成果，避免过于苛刻或挑剔。同时，我们也要勇于接受他人的意见和建议，不断完善自己的代码。其次，要注重沟通和协作。在Code-Review过程中，我们要积极与团队成员进行沟通和交流，共同讨论和解决问题。通过团队协作，我们可以更好地发现问题、解决问题，提升代码质量。

总之，Code-Review是一种非常有效的代码质量控制手段。通过它，我们可以发现潜在问题、统一编码规范和风格、提升团队成员的技能水平，从而实现从混乱到秩序的蜕变，让代码质量得到飞跃式的提升。因此，我们应该重视Code-Review在软件开发过程中的作用，并积极推广和应用它。

本人文章容灾备份，最初发布于这里。如有需要，请移步原始文章进行讨论。