1. 代码审查概述
1.1 什么是代码审查
对计算机源代码系统化地审查,常用软件同行评审的方式进行,其目的是在找出及修正在软件开发初期未发现的错误,提升软件质量及开发者的技术
1.2 为什么要做代码审查
- 可以帮助提高代码质量:代码审查的初衷是为了发现代码的问题并且修正,让系统的缺陷更少,更加容易维护
- 上下文共享:方便团队协作,使不熟悉该模块的团队成员对模块有一定了解
- 帮助新人快速融入项目
- 帮助开发人员成长
- 帮助影响力的建设
但是虽然代码审查有着很多的好处,但是不是所有的团队都会进行代码审查,因为代码审查需要付出专门的时间和精力,对于一些时间周期比较紧急的项目来说,就会放弃掉这个部分;而且当对代码质量要求更高时,可能会对团队成员的要求更加的高,这就有可能引发团队成员间的不适,尤其是一些较为苛刻的修改意见;
1.3 什么时候做代码审查
代码审查的时间应该是及时的,即修改与审查之间的时间间隔不宜过大;一般存在有代码变更的时候就可以进行代码审查,这里的代码变更我们可以认为是已提交到远程仓库的代码,对于一些比较大的改动,则应该选择在未提交到远程仓库时就进行代码审查;
1.4 常见的代码审查工具
一般整个项目的代码可能会有几十万甚至上百万行的代码,所以我们不可能一次性全部审查完,上面我们也提到了,代码的审查一般是针对于那些我们修改过的代码来进行的,所以对于工具的选择,第一点就是能够展示出哪些位置的代码修改过,这里就有我们很常使用的SVN,Git工具来很直观的展示代码的变动;除此之外,Gerrit便是一款比较常见的代码审查工具,再有便是我们常用的IDEA同系列的Upsource;下图就是他们之间的一个简单对比:
2. 代码审查流程
2.1 范根检查法
范根检查法是一种非常正式的审查流程,会带来一定的开销,在日常的场景中很少使用,但是对于资料非常重要,代码缺陷零容忍的场景,这种结构化的审查就很有必要
- 规划:当要开始代码审查的时候,首先确定参加的成员,准备物料以及参加会议的时间地点
- 概述:针对要审查的代码做简要的描述,并且分配参与人员的角色
- 准备:参与者要参加审查的项目,记录下发现的问题和疑问,并为自己的决策做好准备
- 审查会议:参与者一起重新探讨需要检查的项目,以便真正发现错误
- 调整修复:对于一些不重要的位置的代码问题,修复之后直接进行跟进即可;针对于重要位置的代码问题,在修复后需重复以上五个步骤,以保证修复的可靠性
2.2 轻量级审查流程
- 结对编程:在编程的时候便和代码审查者结对,在编程的过程中,代码审查者便会对代码进行审查,并且提出相关的建议,这种方式适合团队有新人加入的时候,可以快速给新人分享上下文,并且进行对应的实践
- 同步代码审查:编码在同步或者一定的时效内就进行审查,相对于结对编程,是在代码完成后再进行的审查,这种代码审查的方式适合于编码在同一个地点的团队,方便规定审查时间
- 异步代码审查:借助代码审查工具,来发起代码审查的请求,然后去处理其他的问题,等待代码审查者一些回复以及反馈
3. 代码审查的关注点
3.1 编码风格
常见的一些编码风格就类似于:命名时驼峰还是下划线;缩进是空格还是Tab,一行代码的最大长度具体是多少等等等等;编码风格的审查,可以通过一些编码工具插件来完成,类似于阿里巴巴代码规范;在提交仓库时,可以使用Gradle工具,来进行自定义配置,并配置到git的一个提交钩子上面;
- 命名风格:
- 不以下划线或美元符号开头或结束
- 类名Upper Camel Case
- 方法名、参数名、成员变量、局部变量都统一使用lower Camel Case风格
- 常量命名使用全大写,单词间用下划线分隔
- 包名统一使用小写
- 禁止拼音命名
- 禁止不规范的缩写
- 常量定义
- 禁止未定义的常量
- 长整型赋值需要使用大写L后缀
- 变量值可穷举,考虑使用枚举
- 代码风格
- 采用四个空格缩进,禁止使用Tab字符
- 单行字符数限制不超过120个
- 换行符使用Unix格式
- 运算符的左右两边都需要添加一个空格
- 保留字与括号之间都必须加空格
- 多个参数逗号后需要加空格
- 左大括号前不换后,左大括号后换行;右大括号前换行,终止的右大括号后换行
- OOP规约
- 静态方法/变量使用类名进行访问
- 复写方法需要增加@Override注解
- 禁止过时方法/类的使用
- 包装类型比较使用equals方法
- 分支控制
- switch中,每个case必须使用continue、break、return终止或者注释说明到哪个case终止
- switch需要对字符串进行非空判断
- 分支逻辑使用大括号
- 注释
- 类/方法注释使用javadoc规范
- 方法内部单行注释,使用//在需要被注释的语句上方单独起一行
- 无用代码删除,而不是注释
3.2 命名规范
起名字一直是我们程序员你的一大纠结点,尤其是对于新入程序员来说;而命名规范所关注的就是我们所需要命名的位置是否有一个好的、恰当的名字:比如方法名,字段名,类名,模块名等等,好的命名可以直接揭示出代码的意图,提高代码的可读性,为后续的维护支持提供更好的支持
我们起名字就需要注意:名字,足够长以揭示意图,但又不太长难以阅读
- 使用有意义的名字
- 范围越大,命名越长;范围越小,命名越短
- 避免使用非约定的缩写
- 使用一种自然语言来命名,比如英语
- 使用对应领域的专业名称,这部分建议在命名时不确定,可以寻求翻译软件的帮助
- 不好命名,考虑类、方法职责过多,是否需要拆分重构
- 一致性问题:不管使用哪种规范,团队内一定要达成一致
3.3 功能性
这一点主要从用户的角度来分析我们的代码是否复合用户的预期,这里的用户不仅仅是包含我们的终端用户,同时也包括我们的开发者
- 验证输入输出、流程是否正确
- 边界是否考虑并处理妥当
- 是否有高并发的安全问题
3.4 测试覆盖
测试的定义:在规定的条件下对程序进行操作,以发现程序错误,衡量软件质量,并对其是否能满足设计要求进行评估的过程
测试覆盖是我们编写的代码是否都有对应的测试来保证其是符合预期的,以保证我们上线的软件功能是正常的,这里不是指没有BUG,而是说功能正常即可
3.5 复杂度
当我们的代码越复杂,就会导致我们的代码可读性越低,越容易引起缺陷,导致模块的内聚性降低,同时不便于我们后续对代码的维护升级;
复杂度的度量一般我们是利用圈复杂度来判断的,其判断公式为
V
(
G
)
=
判
定
节
点
数
+
1
V(G) = 判定节点数 + 1
V(G)=判定节点数+1
其中属于判定节点的有:if语句;while语句;for语句;switch-case语句中的case数量;try-catch语句中的catch语句;与、或、三目运算符这些都算是判定节点,因为在判定节点,我们的代码分支就会越来越多,这也就导致我们的代码越来越复杂;
所以根据上面的方法算出来的复杂度,对于一个方法来说,它的圈复杂度大于10时,我们就认为它的复杂度过高 我们就需要做一定的优化,以降低代码复杂度
对于复杂度优化,一般有以下几种方法:
- 方法抽取
- 反向表达
- 单一职责
- 使用多态
3.6 注释
注释虽然并不会直接参与实际代码的运行,但是,一个良好的注释能够进一步提升代码的可读性,帮助其他开发者去理解代码作者的意图,并且能帮助开发者正确使用对应的代码或工具;这里要注意:再好的代码风格,命名规则,也不及自然语言的注释,但是这也不是说每一行都要写注释:
- 对外、公共的接口
- 模块或者系统的描述
- 宁缺毋滥,这里是说,哪怕不写注释,也不要写模糊的注释
3.7 设计
设计本身不会影响代码的功能性,但是一个好的设计,一方面会提升系统的稳健性,另一方面会对我们代码后续的一个维护升级提供便利,且对程序的扩展会相对友好,这里我们有以下几种指标来判断一个设计
- 是否足够解耦
- 是否可以使用一些设计模式
- 是否可以应对一些变化
- 是否过度设计
3.8 安全性
主要包括数据安全性和程序运行安全性两个方面,足够的安全性是保证程序能够运行的一个基础,避免被一些攻击带来的数据的一些泄露、程序的异常和资源消耗异常,一般我们从以下几个方面来考虑
- 源码库是否包含凭据
- 敏感信息受否加密落库
- 输出是否安全
- 输入是否安全
- 权限管控是否准确
- 返回值谨慎使用null
当然,互联网发展了这么多年,总会有一些自动化的工具能够帮助我们完成大部分的工作,例如sqlmap,主要针对SQL注入的一个工具;owasp,主要针对一些依赖进行安全检测;Web Cruiser则属于一个比较综合性的检测工具;
虽然有很多的工具来帮助我们搜寻一些漏洞,但是,还是需要我们自己在代码开发的时候更加的严谨,提高代码安全性
4 一些小细节
4.1 被审查代码的作者应注意
- 变更描述要足够清楚,避免因为描述不清楚而再次浪费时间
- 单次改动不要过大,会导致一次审查量过大
- 积极接受反馈,毕竟每个人的考虑总会有漏洞
4.2 审查代码者应注意
- 一定要提前设置好度量标准,不要每天都变
- 注意审查的速度,不要一拖再拖
- 要给予良好的反馈,不要总是给负反馈,对好的代码要提出表扬,注意沟通方式