代码检视(Code Review)是指软件开发人员在完成代码设计、编写、调试后展开的个人或群体性的代码阅读过程,代码检视的目的是发现代码中的设计问题、格式问题、逻辑问题、语法问题等,从而保证代码的高质量交付。从软件工程的角度讲,在代码检视阶段发现代码问题的成本是低廉的,所以严格认真的执行代码检视过程,是提升产品质量,降低产品维护成本的有效手段。

一、代码检视的指导思想

在IBM的一篇文章中,对高效的代码检视方式和方法进行了分析和总结:11 个高效的同行代码评审最佳实践,它主要提出了以下几个观点:

(1) 一次评审少于 200–400 行的代码。
(2)目标为每小时低于 300–500 LOC 的检查速率。
(3)花足够的时间进行正确缓慢的评审,但是不要超过 60–90 分钟。
(4)确定代码开发者在评审开始之前就已经注释了源代码。
(5)为代码评审和获取制度建立可定量化的目标,这样您才能改进流程。
(6)使用检查列表,因为它可以极大地改进代码开发者和评审者的作品。
(7)确认缺陷确实得到修复了。
(8)培养良好的代码评审文化氛围,在这样的氛围中搜索缺陷被看做是积极的活动。
(9)警惕“老大”效应。
(10)最少评审一部分代码,就是您不能评审全部的代码,以从 Ego Effect 中受益。
(11)采用轻量级,能用工具支持的代码评审。

其中,我比较认可的一句话是:管理员必须不断地维持这样一个念头,即搜索缺陷是好的事情,而不是糟糕的,缺陷密度与开发员的能力并不是挂钩的。记住对一个团队来说,缺陷,尤其是团队成员所引入缺陷的数量不应该被回避,也不应该用作能力的评价参数

代码检视作为一个勘误的过程,核心目的是提升代码质量,同时提升开发人员的能力,而不是批斗和责备开发人员,要积极引导参与人员的代码检视态度,把代码检视作为提升开发人员能力的一项活动来进行,我想这样才能达到代码检视的核心目的。IBM的这篇文章倡导进行轻量级的代码检视,即高效的利用IDE集成的代码检视工具开展代码检视工作。我不完全认同这个观点,重量级的代码检视(召集会议室集体检视),确实会消耗更多的人力和时间去发现问题,但重量级的检视同时也构建了参与人员相互学习,相互交流的良好氛围,而轻量级的代码检视(利用检视工具)基本上无法达到这个目的,同时,代码检视需要包含对设计的检视,更多的时候需要代码开发者讲解其设计思路,这个也是工具检视无法达到的。所以何时开展重量级的检视,何时又利用工具进行检视,可以根据项目的进度,团队综合能力和素养来决定,个人觉得需要综合利用,重量级的检视目的是为了开发人员以后更少的引入缺陷,从而变重量为轻量。

二、代码检视的内容

既然有了代码检视的指导原则,那么在代码检视中,我们究竟检视些什么内容呢?看看别人的总结:

1、与详细设计的一致性
只要将检视的代码对照详细设计进行比较就很容易检查出代码是否和详细设计一致,采用逐行逐字阅读进行比较的方法进行。
2、头文件检查
头文件检查主要关注以下方面:
(1) 是否包含有多余的其他头文件
(2) 头文件是否内聚,即是否多个模块共用一个头文件
(3) 头文件内的内容是否清晰,是否分类排放好并给出了足够的注释
(4) 是否使用了象 #ifdef __LIST_H__ 之类的宏定义保证头文件不被重复引用
3、宏定义检查
(1) 宏定义中有参数和表达式时,参数和表达式是否都用括号括起来了。例如:
#define ADD(a, b) (a + b) //正确的应该是 ((a) + (b))
这个定义中就没有将参数a和b括起来,如果使用时a和b是表达式的话,就会因为运算符顺序问题而出问题。
(2) 续行符\是否使用正确
4、常量
常量方面主要检查的主要问题如下:
(1) 常量是否使用了宏来进行定义
(2) 程序中是否存在魔鬼数字
(3) 16进制数据是否在前面加上了0x
(4) 常量是否来自规格
(5) 不来自规格的常量的值是否合理
5、全局变量与共享变量需要检查的主要问题如下:
(1) 全局变量是否必须的,是否可以改成局部变量
(2) 是否有全局范围内变量和局部范围内变量重名情况
(3) 是否有多个任务访问共享变量,是否进行了有效的保护
(4) 当全局变量只限于本文件内使用时,是否定义成静态的
(5) 多个任务读写共享变量时,是否可以将读写操作封装成独立函数,而不是在每个模块里都进行加锁解锁操作
6、 静态变量和函数
静态变量和函数检视时主要问题如下:
(1) 静态变量的使用是否正确
(2) 每次使用静态变量时是否需要重新初始化
(3) 对不需要重新初始化的静态变量在多次使用后是否有溢出的问题。
(4) 文件内部使用的函数是否要定义成静态的
7、 数据结构
数据结构方面考虑的主要问题如下:
(1) 数据结构里的成员类型定义是否正确
(2) 结构体里面变量顺序安排是否合理,数据是否对齐
(3) 是否存在冗余未用的成员变量。
(4) 类里面是否有私有变量和私有函数放到了公有的定义里去了
8、初始化
初始化考虑的主要问题如下:
(1) 变量使用前是否需要初始化
(2) 类的构造函数中是否对需要初始化的成员都进行了初始化
(3) 数组的初始化是否正确
(4) 内存或数组在每次使用前是否需要初始化清零
(5) 多个变量初始化赋值时是否存在顺序问题
(6) 静态变量和全局变量的初始化是否存在初始化顺序问题
9、字符串
(1) 字符串是否以’\0’结尾
(2) 字符串是否会超长
(3) 字符串使用的空间大小是否存在差1问题
(4) 使用字符串指针时,指向的位置是否存在差1问题
(5) 字符串指针是否可以为空,为空时会有什么现象?
(6) 字符串内容为空(即第一个字符为’\0’)时会发生什么现象?
(7) 字符串中如果有转义字符“\”字符时,是否正确地写成了“\\”
(8) 在对字符串进行拷贝或连接操作时,是否对空间大小进行校验?
10、输入检查
输入校验需要检视的主要问题如下:
(1) 函数参数是否需要进行检查
(2) 从文件读取的数据是否进行校验
(3) 使用全局数据时是否需要进行校验
(4) 通信收到的数据是否需要进行校验
(5) 从消息中接受到的数据是否需要进行校验
11、边界条件
凡是牵涉边界条件的地方都需要进行边界检查,以下的一些问题供参考:
(1)循环变量上的边界是否正确
(2)变量的取值是否有边界条件限制,边界是否给出并书写正确
(3)空间边界,如内存大小,数组大小是否正确,是否存在差1和越界情况
(4)数据结构边界,如链表的头一条记录和最后一条记录等边界情况
(5)服务器连接数量最大是多少
12、内存分配和释放
内存分配方面需要检查的有以下几点:
(1) 分配的大小是否正确,是否分配了过大的内存或者分配的内存大小不足,分配的内存大小是否存在差1错误
(2) 内存分配是否经过判断或者进行异常处理
(3) 重新分配一块内存时,是否将原有内存释放
(4) 分配的内存是否需要初始化清零
(5) 是否有在大循环中不断分配内存导致可能出现系统内存不足情况
释放方面需要检查的有以下几点:
(1) 所有的分支路径上是否将分配的内存进行了释放
(2) 是否将已经释放的内存重复释放
(3) 释放的是否是空指针
(4) 释放多块内存时是否存在释放的先后顺序问题
使用realloc()时要考虑以下几点:
(1) 新增空间是否需要初始化清零
(2) 是否还有指针指向老的内存块,并在realloc()后使用指向老的内存块的指针。
13、类型转换
类型转换的检查有以下问题供参考:
(1)类型转换是否采用安全的转换机制
(2)当采用强制转换时是否会出问题
(3)signed 和unsigned转换是否存在问题
(4)是否将小空间的类型转换成了大空间的类型
(5)类型转换是否会造成截断、溢出或越界
14、数组使用
数组的使用也是很容易出错的一种,不幸的是现在还没有足够好的方法能保证数组越界一类的问题得到完美的解决,所以通过对数组的检视来保证质量就很重要了,下面给出检视数组的一些建议:
(1)类型是否正确
(2)多维数组是否数据存放顺序正确
(3)数组使用时是否会越界,空间大小是否存在差1错误
(4)作用域是否正确
(5)数组大小是否太大导致浪费
15、指针使用
指针在C/C++中是使用最广泛的一种语法,指针使得语言的功能强大起来,但也给程序质量带来了很大麻烦,使用指针时是极易出错的,可以说C/C++代码中的缺陷大部分都与指针有关,下面给出检视指针的一些问题参考:
(1)指针是否初始化
(2)指针类型定义是否正确
(3)使用前是否申请了内存
(4)引用是否正确,是否引用了释放掉的空间
(5)指向的空间是否正确
(6)是否存在使用野指针现象
(7)释放后再使用时是否需要重新初始化
(8)是否使用了空指针,函数指针是否为空就被调用
(9)指针是否需要校验
(10)指针进行类型转换时是否会引起问题
(11) 指针地址运算是否有误,在地址相加时是否考虑了相加的数字要乘以指针类型所占空间的大小。比如int *p; p+1相比p的大小不是大于1,而是大于一个整数所占空间的字节数。
16、函数
函数方面的一些检视建议如下:
(1)函数调用的参数传递是否正确,
(2)是否有形参和实参使用错误的问题,
(3)函数的返回值和输出是否需要校验,
(4)调用的函数是否对全局数据产生影响
(5)函数功能是否单一,是否在函数里处理了多个不同的功能
(6)函数参数是否需要定义为const
(7)函数是否过长(一般以不超过200行为宜)
17、系统和标准库调用
调用系统函数和库函数时,以下一些检视建议供参考:
(1)系统调用是否正确,调用参数设置是否正确
(2)是否按照标准文档中的要求和注意事项进行了调用
(3)对于存在BUG的系统函数是否采取了规避措施进行调用
(4)对调用系统函数是否需要在调用前进行了输入校验
(5)调用后是否需要对输出进行校验
18、判断循环条件
在程序中的判断和循环条件中,也存在着一些有时通过测试难以发现的问题,主要的检视建议如下:
(1)逻辑运算符是否正确,如| 和||,&和&&运算符是否搞混淆掉或键盘失误写错,
(2)逻辑等号==是否误写成等号=
(3)运算符顺序是否正确,运算符| 、&,||、&&,=、== 的运算顺序需要特别注意
(4)循环判断中的表达式是否正确地使用了括号将运算顺序区分开,并增加可读性
(5)表达式运算是否存在逻辑上的错误
(6)对浮点数是否误用了精确相等进行比较
(7)循环变量是否进行了初始化
(8)循环的中止条件是否在某些情况下无法到达而造成死循环
(9)循环的边界上是否会造成问题
(10)判断条件是否会恒真或恒假

19、计算
计算错误也是程序中经常遇到的一个问题,大部分计算错误可以经过测试发现,但并不是所有的计算错误都可以很容易通过测试来发现,以下的一些问题供检视时参考:
(1)计算表达式或公式是否书写正确,需要逐字符地进行确认没有输入错误
(2)表达式中运算符顺序是否书写正确,同优先级运算符运算时是否存在自左至右结合或自右至左结合运算结果不同的问题
(3)是否需要使用括号来保证运算顺序的正确性和增加程序的可读性
(4)是否存在计算溢出情况,如两个整数相乘结果超出整数最大范围等情况
(5)截断误差和舍入误差是否会引起问题,误差是否会累积下去导致误差越来越大?
(6)是否存在除零问题(即零做分母的问题),或者两个整数相除结果得到零然后再和其他整数相乘。
(7)是否存在某个变量会累积增加导致长时间运行后的溢出
20、资源释放
资源释放方面的一些检视建议如下:
(1)所有的资源是否都进行了释放
(2)要检查是否存在某条路径遗漏了释放
(3)打开文件是否关闭了,信号量是否释放,句柄是否关闭,锁资源是否释放,是否存在死锁问题
(4)全局的资源是否存在随时间累积增加不减少的问题?
(5)其他各种资源如网络socket等是否在各条对应路径上进行了关闭
(6) 类的析构函数中是否对类中需要释放的成员进行了释放
21、效率
从空间和时间效率方面一些检视建议如下:
(1)多个if判断是否可以改为switch或if…else if结构,例如if(res<0){…}
if(res>1){…} if(res>=0){…}改为if…else if…else就可以减少判断次数
(2)多个相同处理的case语句是否归到一起
(3)在多重循环中,最忙的循环是否在最内层
(4)循环体内的判断语句是否可以移到循环体外

这个就比较坑爹了,这么多框框条条款款,谁记得住呢?我觉得这个问题需要这样理解,上面给出的是代码检视中需要去关注的问题,因为这些问题是开发人员常常容易跳的坑。这些要点可以过几次,在脑子中形成印象,将其应用到实际的代码检视中去,长此以往,无招胜有招,不用刻意去记忆这些条款。只要知道了什么是优秀的代码,那么看见不一致的写法时,就需要问自己这样写有什么好处,如果没有,那么这个就是问题代码,至少可以说是非主流代码。写非主流代码的可能是高手,但是在产品中,特别是大型软件开发项目中,不需要这些非主流,这些“高端”代码会给后期的变更和维护带来巨大痛苦。

三、代码检视的重要性

之所以在本文最后才提这个东西,是因为想再次强调这个东西,不多言语,看张图片,一切尽在不言中。