什么是代码评审:
代码评审也称代码复查,是指通过阅读代码来检查源代码与编码标准的符合性以及代码质量的活动。
通过工具来进行code review不在本次讨论范围内。
评审的内容:
编码规范问题:命名不规范、magic number、 System.out……
代码结构问题:重复代码、巨大的方法和类、分层不当、紧耦合
工具、框架使用不当:Spring、Hibernate、AJAX
实现问题:错误验证、异常处理、事务划分、线程、性能、安全、实现过于复杂、代码可读性不佳、扩展性不好
测试问题:测试覆盖度不够、可测试性不好
代码评审不负责检查功能、逻辑是否正确,这些要靠单元测试和QA工作来解决
代码评审的好处:
提高代码质量
在项目的早期发现缺陷,将损失降至最低
评审的过程也是重新梳理思路的过程,双方都加深了对系统的理解
促进团队沟通、促进知识共享、共同提高
交叉评审——代码走查:团队成员互相检查代码
参与者可以是任意两个组员,或开发组长分别与每个组员结对进行
时机可以选择在下班前半小时,对当天改动的模块进行评审
代码作者讲解如何以及为何这样实现、评审者提出问题和建议
每次解决的问题要记录到SVN注释或JIRA
每次评审不要贪多,如下图所示:当一次评审超过400行代码时,能发现缺陷数显著降低——事倍功半
会审:以项目为单位,召开专门的代码评审会议
参与者:包括项目组全体成员,其它组的开发组长也应尽量参加
时机选择:开发进行到某一阶段时,对共性问题进行总结,对好的做法进行提炼和推广
会前准备工作:
组织者应通知各参与者本次评审的范围
参与者阅读源代码,列出发现的问题、亮点,汇总给组织者
准备工作要细致,需要给出问题详细描述以及相关代码在SVN上的URL地址等
评审代码的选择:
最近一次迭代开发的代码
系统关键模块
业务较复杂的模块
缺陷率较高的模块
会议议程:
如果是第一次会议,先由该项目开发组长做整体介绍
参加者依次发言,结合代码讲解发现的问题
每讲完一个问题,针对其展开讨论,每个问题控制在10分钟以内
如果问题不多,还可以安排该组成员对最近开发的代码进行地毯式的讲解和排查;或者针对某个方面对整个项目做评审,例如性能、安全性或测试
会后总结:
把会上提出的所有问题、亮点及最终结论详细的记录下来,供其他团队借鉴
未能讨论清楚的问题,会后解决
实行代码评审制度前的准备工作:
架构师提供开发规范、指南,为代码评审提供依据
建立起单元测试规范,否则无法达到测试覆盖度的要求、难以修正发现的问题
最好有样例代码库作参照,以提高代码评审的可操作性
提供评审案例:用评审前的代码与评审后优化的代码做对比
问题跟踪:对评审中发现的问题代码应加以跟踪,确保问题得以解决,防止复发
评审到什么程度:
进行全面的代码评审成本较高,也没有必要
对发现的问题要本着集体代码所有制的观点和就事论事的原则,因此建议把代码质量与团队绩效(而不是个人绩效)挂钩
下面是一个代码评审用的模板
分类 | 规则 | |||||||||
命名 | ||||||||||
文件名 | 1 | 第一个单词的首字母小写,其余单词的首字母都大写。 | ||||||||
类名 | 2 | 以大写字母开头,每个单词的首字母都大写。 | ||||||||
方法名 | 3 | 第一个单词的首字母小写,其余单词的首字母都大写。 | ||||||||
4 | 使用动词或动词性短句。 | |||||||||
变量名 | 5 | 成员变量加前缀 m_ | ||||||||
6 | 静态变量加前缀 s_ | |||||||||
7 | 全局变量加前缀 g_ | |||||||||
8 | 指针的变量名前以“p”标识。 | |||||||||
9 | 指针变量作为成员变量时,以“m_p”开头。 | |||||||||
魔数 | 10 | 尽量用枚举或宏标识一些常量,不要在代码中直接写数字。 | ||||||||
内存管理 | 11 | new出来的指针必需用delete删除,删除后立即赋空。 | ||||||||
12 | new申请的内存,应该用if(p == NULL)判断内存是否申请成功。 | |||||||||
13 | 使用指针前应判断指针有效性。 | |||||||||
14 | 尽可能在定义变量的同时初始化该变量,指针如果不立即初始化,应该赋空。 | |||||||||
15 | 成员变量不要使用引用。 | |||||||||
函数 | 16 | 当以对象作为函数参数时,可以传入变量的引用,如果在函数体内不修改这个参数,形参就应该声明成const。 | ||||||||
17 | 当不允许函数修改成员变量时,应将函数声明成const。 | |||||||||
18 | return 语句不可返回指向“栈内存”的“指针”或者“引用”。 | |||||||||
19 | 函数不要写得太大,尽量控制在50行以内。 | |||||||||
20 | 函数返回值非void时,要保证函数中的每条路径都有返回值。 | |||||||||
版式 | 21 | 每个文件的最后都要加一个空行。 | ||||||||
22 | 每个类声明之后、每个函数定义之后都要加一个空行。 | |||||||||
23 | 函数体内,逻辑上密切相关的语句之间不加空行,其它地方应加空行分隔。 | |||||||||
24 | if、for、while、do 等语句自占一行,执行语句不得紧跟其后。不论执行语句有多少都要加{}。这样可以防止书写失误。 | |||||||||
25 | 代码行最大长度宜控制在 70 至 80 个字符以内 | |||||||||
26 | 长表达式要在低优先级操作符处拆分成新行, 操作符放在新行之首 (以便突出操作符) 。拆分出的新行要进行适当的缩进,使排版整齐,语句可读。 | |||||||||
27 | 尽量避免名字中出现数字编号,如 Value1,Value2 等,除非逻辑上的确需要编号。 | |||||||||
28 | 如果代码行中的运算符比较多,用括号确定表达式的操作顺序,避免使用默认的优先级。 | |||||||||
零值比较 | 29 | 布尔变量与零值比较时,if (flag == true)表示flag为真,if (flag == false)表示flag为假。 | ||||||||
30 | 整型变量与零值比较时,应该将整型变量用“==”或“!=”直接与0比较。不可模仿布尔变量的风格。 | |||||||||
31 | 浮点变量不可用“==”或“!=”与任何数字比较。 | |||||||||
32 | 指针变量与零值比较时,应该用if(p == NULL)。不要用if(p == 0),容易与整型变量混淆;也不要使用if(!p),容易与布尔变量混淆。 | |||||||||
其他 | 33 | 图片的设置用GetImagePath("不带路径的文件名") 例如:GetImagePath("DialogBase_1.PNG") | ||||||||
34 | 删除多余的文件,多余的图片,多余的代码 | |||||||||
35 | 注释参照0201003_TM_ソースコメント規約 | |||||||||
36 | 画面效果参照对应的寸法图效果 | |||||||||
37 | XGA/SVGA的对应 |