1、什么是CodeReview?
Code Review(CR)即代码评审,又名代码走查,是一种通过复查代码来提高代码质量的过程,一般体现在一个团队的开发过程中。CR要求团队成员有意识地、系统地检查彼此的代码,从而验证需求、发现错误,同时指出其中不合规范的“低质量”代码,从而提高整个团队的代码质量。
一次 CR 可以是一次 Commit,也可以是一次 Merge Request。因此,实践课系统支持团队内部的 MR 评审以及 Commit 评审,供大家学习和交流。
2、为什么要CodeReview?
(1)旁观者清。
对于同一段业务代码,由于看待问题的角度不同,评审者可能会比开发者更容易发现其中的问题,或是找到更有效的解决方案,共同维护团队的代码质量。
提高代码质量和可维护性, 可读性等。
查漏补缺, 发现一些潜在的问题点等。
最佳实践, 能够更好更快的完成任务的方法。
知识分享, Review他人代码时, 其实也是一个学习的过程, 自己也会反思&总结。
(2)快速了解业务。
理想状态下,团队中的每个人都需要对整个项目的各个部分都很熟悉,当然,在项目很大时这是不现实的。通过代码审查至少可以让每个人了解更多的业务模块,同时也能达到人员互备的目的。
人员互备:通过 CR,评审者也相当于参与了这次开发,相当于一种人力“备份”,当你休假或正在忙别的需求的时候,这时“备份”或许就能帮上你的忙了。
(3)开发者能够获得什么?
对需求的理解得到加深。
表达能力得到加强。
逻辑能力得到训练。
心理承受能力得到提高。
(4)评审者能够获得什么?
快速上手业务需求和全局的架构。
统一大家约定俗成的代码风格。
优秀的设计思路和业务逻辑。
3、CodeReview 的原则。
(1)CR 是必要的,但也需要结合团队现状。
当你的团队开发任务极其紧张,再耗费一部分人力去进行 CR,是不明智的。
(2)所有的代码都应被赞成。
因为团队代码库的每一次改动(Change List ,以下简称 CL),都必定会提高当前系统的整体质量,即使这个 CL 并不完美。
(3)CR 不应该追求完美,而应追求持续改进。
要知道,没有完美的代码,只有更好的代码,“慢即是快”。
(4)CR 不是挑刺,更不是证明谁的能力更强。
CR 是为了提高整个团队的能力,而不是针对个体设置的检查“关卡”,仅具有指导意义。
(5)评审者也需要对这个 CL 负责。
CR 不应设置奖惩机制,即便是有,也是对评审者和开发者同时的奖励或处罚。
(6)时刻保持谦虚。
无论是评审者还是开发者,都可以在 CR 中提升自己的能力。
(7)合理解决问题
解决冲突难以达成共识时,需要面对面或者拉起更大的团队讨论,带上leader。
4、在一次CodeReview中我需要关注什么?
(1)git 提交规范
(2)代码风格
a. 可读性
衡量可读性, 有很好的实践标准, 即 Code Review 时能否非常容易的理解代码逻辑, 如果不能, 那意味着代码的可读性要进行改进。
b. 命名
描述是否准确。不建议使用生僻单词。
命名格式是否与团队风格一致。
c. 函数体长度/类长度
函数体太长,不利于阅读。一般建议不超过50行。
类太长,有可能违背“单一职责原则”。
d. 参数个数
参数不要太多。一般不超过5个;5个以上建议使用对象。
e. 注释
恰到好处的注释,能够帮助评审者更好地理解函数体和类。
(3)架构/设计
a. 单一职责原则
一个类只做一类相关的事情。
一个方法,最好只做一件事情。
b. 行为是否统一
校验处理是否统一。
数据处理是否统一。
错误处理是否统一。
…
c. 代码是否污染
代码是否对其他模块有强耦合。
d. 是否有重复代码
检查是否将可复用的方法或组件抽取出来。
e. 开放-封闭原则
代码是否具有良好的可扩展性。
f. 代码健壮性
核心数据有没有强制校验。
边界值有没有考虑得当。
有没有潜在的bug。
有没有内存泄露。
有没有循环依赖。
…
g. 是否考虑了错误处理
有没有很好的 Error Handling。
h. 面向接口编程
检查业务的实现中有没有进行合适的抽象。
i. 效率/性能
对用户使用频率高、资源消耗大的业务部分是否处理得当。
关键算法的时间复杂度。
有没有潜在的性能瓶颈。
j. 代码重构
新的改动是打补丁,让代码质量继续恶化,还是对代码质量提升有帮助。
k. 时间粒度。
一个Review单元的时间应控制在10~20分钟,如果超出这个时间,那么这段代码本身可能在业务划分或代码实现上,是存在问题的。