技术团队如何做好 CR(CodeReview)
Code Review,代码复查,以下简称 CR
一、好处:
参考:https://mp.weixin.qq.com/s/n1B0wLicwQByYslz6hYwnw
代码有这几种级别:1)可编译,2)可运行,3)可测试,4)可读,5)可维护,6)可重用。通过自动化测试的代码只能达到第3)级,而通过Code Review的代码少会在第4)级甚至更高。
不断提高团队的下限——良好的程序设计总是优于CR,做好设计才能减少review工作量。
二、谁参与?
编码者;
其它开发人员,由编码者自行邀请1-2位;
其它感兴趣人员:如测试工程师。
三、在什么时候,针对什么内容进行Review?
(时机)在开发过程中,只要有清晰功能节点代码提交时就行(提倡经常简短性的Review),(内容)针对业务的核心代码逻辑——比较零散的就没必要了。
因此,不能等到迭代结束后才进行,有两个原因:
1)、项目在不断迭代,一般来说都很难有机会“等有时间进行优化”;
2)、挤压的待优化点太多,优化后容易造成系统性风险,对已上线的版本影响太大。
四、从哪些方面进行Review?
Step1、在人工CR之前,先借助插件扫描并修改完成,主要扫除代码规约性问题,如代码缩进、变量定义等等..
自检工具,Java的如:Alibaba Java Coding Guidelines;针对 IOS 的 Sonar代码质量检测系统;针对 web前端(HTML、CSS、JS)也挺多。
以Alibaba Java Coding Guidelines 为例:
插件安装,File->Setting->Plugins
插件使用,项目扫描:
已有的代码扫描出很多问题怎么办?
先不理会,在以后有修改到相应业务代码时顺手而为,这样不存在太大风险。
Step2、再进行人工CR,侧重 编程素养、业务逻辑、架构设计、单元测试、性能、安全。
具体项,参考《附:Checklist》,更多参考书籍:《重构-改善既有代码的设计》 和 《Clean Code》(代码整洁之道)
五、规则:
1、不要太正式,时间要短(5分钟讲解,5分钟提意见就好)。
2、由编码者讲解,其他人员随读理解、优化建议(编码者以注释方式:标注优化建议)。
3、一次CR最好控制在200行左右,最多不超过400行(大脑一次只能有效处理这么多信息,超过400 行找到缺陷的能力减弱) ;
4、CR过程中,建议将可优化处先标注出来(如:“// CR:抽取重复代码 ”,不着急修改);
5、CR完成后,时间充裕可现场讨论修改优化。
六、心态
要富有同理心。
在review和被review的时候心态要好。如果是代码review者,发现问题后最好跟代码编写人明确问题所在,帮他理清修改思路。被review 者也要虚心接受review 结果,毕竟代码检查也是一项耗心耗力的事情,尊重rev者的付出,根据结果进行辩证性修改。这两个过程,对彼此两个人都有提升。
写代码不易,改代码更难。珍惜每一个给我提的意见,因为每个意见可能是对方走过弯路吃过亏的经验教训。珍惜每一次给他人review 代码的机会,可以让我从发现的问题中得到警示。
附:Checklist
常规项
代码能够工作么?它有没有实现预期的功能,逻辑是否正确等。
所有的代码是否简单易懂?
代码符合你所遵循的编程规范么?这通常包括大括号的位置,变量名和函数名,行的长度,缩进,格式和注释。
是否存在多余的或是重复的代码?
代码是否尽可能的模块化了?
是否有可以被替换的全局变量?
是否有被注释掉的代码?
循环是否设置了长度和正确的终止条件?
是否有可以被库函数替代的代码?
是否有可以删除的日志或调试代码?
安全
所有的数据输入是否都进行了检查(检测正确的类型,长度,格式和范围)并且进行了编码?
在哪里使用了第三方工具,返回的错误是否被捕获?
输出的值是否进行了检查并且编码?
无效的参数值是否能够处理?
文档
是否有注释,并且描述了代码的意图?
所有的函数都有注释吗?
对非常规行为和边界情况处理是否有描述?
第三方库的使用和函数是否有文档?
数据结构和计量单位是否进行了解释?
是否有未完成的代码?如果是的话,是不是应该移除,或者用合适的标记进行标记比如‘TODO’?
测试
代码是否可以测试?比如,不要添加太多的或是隐藏的依赖关系,不能够初始化对象,测试框架可以使用方法等
是否存在测试,它们是否可以被理解?比如,至少达到你满意的代码覆盖(code coverage)。
单元测试是否真正的测试了代码是否可以完成预期的功能?
是否检查了数组的“越界“错误?
是否有可以被已经存在的API所替代的测试代码?