也不知code review是从哪年开始流行的,我的职场经历从刚开始完全没有到1对1,再到团队式review
一、Review Meeting
优点:
1.团队内新技术/新观点的交流Meeting、项目开发思路、解决方案讨论,偏头脑风暴式;2.各类项目都适合进行;
缺点:
1.依赖于主持者(项目owner)的素质、时间成本高(为会议上所有人时间的总和);2.集体评审的代码行数有限;
二、Single Review
优点:
1.更偏重与具体的代码评审,人员分散参与,评审代码行数有保证;2.时间自由,reviewer什么时候进行评审时间可自控;
缺点:
1.依赖reviewer的技术水平,代码提交合并前强审核,只适用于重要项目或核心模块;
why
为什么需要code review,其实在任何行业,基本都是大厂带给整个行为最佳实践,code review就是其中一种实践
The biggest thing that makes Google’s code so good is simple: Code Review.
At Google, no code, for any product, for any project, gets checked in until it gets a positive review.
code review的好处可以罗列出很多很多,设计、结构、编码方方面面
代码有这几种级别:1)可编译,2)可运行,3)可测试,4)可读,5)可维护,6)可重用。通过自动化测试的代码只能达到第3)级,而通过Code Review的代码少会在第4)级甚至更高
Code Review主要是让你的代码可以更好的组织起来,有更易读,有更高的维护性,同时可以达到知识共享,找到bug只是其中的副产品
以我个人经验看,code review更多是技术及业务知识的分享,甚至可以相互结合,理论分享与code的结合
比如check list与最佳实践结合
how
code review有点类似TDD,但强于TDD,这儿的强于不是说功能性,而在于落地层面,只要大家坐一起,指点江山,就可以完成了,当然效果另说
怎么更好地落地code review呢?或者说code review需要review些什么?code?
每个团队都有各自的情况,所以并不是随便拿一份review check list对照就做好,至少侧重点不同
比如人家团队人员素质普遍高一些,那人家的checklist可能就少了些基础知识点;团队职责不同,checklist也可能会相应不同,基础架构的checklist肯定跟业务线的不一样,各个不同业务线的也不同,需要根据团队情况制定合适的checklist
极端情况,团队中无人能识别好代码,每次都是流水帐式看代码,那团队人员得流动一下了
如何code review,结合why谈谈一些点
不要挑毛病
这就是上面的图中显示的,尤其团队式review,一群坐在下面的人
1.命名不太好啦2.空格太多了3.方法太长了4.编码没格式化啊5.这循环用lambda一行解决问题6.实现的不是产品要的7.这设计有问题啊
对1~4点,这些纯粹是浪费时间,一个团队的时间是宝贵的,来review这些,极大的浪费
因此需要明确两点:
•Code review 不应该承担发现代码错误的职责•Code review 不应该成为保证代码风格和编码标准的手段
【管理工具化、工具流程化】指导方针,这儿可以引入checkstyle工具,让团队统一code sytle,新人加入团队时的培训指南中,并加入到CI中,检查失败直接构建失败
再引入sonar识别常见质量问题和安全问题,这样提高code review的质量
第5点:这也很典型,从code review层面讲,这也不应该是code review的职责,但从知识分享角度讲,这的确是,怎么办呢?使用流还是经典的for循环最好,如果团队成员对同一段代码有不同的意见,那么开发人员应该如何进行修改,结束审阅,并将代码推送到生产中?
解决这个问题最好能有一套最佳实践标准,明确什么情况使用流式,什么情况使用传统方式,其实这很难,真这样搞最佳实践会成为一本谁也学不完的手册,那只能说“这要看情况”,未尝不可,但需要有前提,团队中需要有一名裁决者来决定最终方案,而不能陷入长时间的争论
好比service能不能跨业务调用dao,这也是无对错,需要是的团队的一致性和最初的决策方案,不必每次code review时无休争论
6~7两点,这是最坑的,浪费了开发时间,也对代码作者造成极大打击,为什么到此时才发现,所以需要在开始前就得对功能设计和架构设计进行review,不能只看结果,得看起始与过程
保证正确性
这是code review的前提条件,如上述的6、7两点,不应该出现,一个优秀的工程师需要能够独当一面,能够在系统角度实现局部的良好设计,通过合理的测试方法论验证结果。能够用合理的数据结构、算法实现功能
在技术驱动的团队里,即使需求很紧急,对于关键的功能,核心成员也会耐心地审视架构设计和实现方案,如何控制熵,如何用更合理的方式实现,如何考虑到未来的变化。技术驱动的团队里,应该持续进行对设计的调整和代码的微小重构与改良,时刻在整个系统的设计和表现(performance)角度审视自己的工作。这也是“系统思考”的核心。大部分的代码的熵增大难以控制,并不是因为没有好的架构,而是因为迭代中忽略了系统性的思考和审视,而是用局部的解决方案解决问题,在反复迭代后,复杂度过高导致控制熵变得异常困难。这是code review比较难解决的
分享
从上面所述,code review虽然能发现代码中的一些错误,但不应该是他的核心价值。正好在《DDD总结》[1]中所述,“降低代码复杂度”是所有方法实践论的终极目标。降低复杂度、易于扩展是我们的目标。那么code review也应该是为实现这个目标的手段,因此code review需要去review设计的合理性(如实现方法,数据结构,设计模式,扩展性考虑等),是否存在大量重复代码等
如何达到这些呢?需要发挥团队力量,三个臭皮匠顶过一个诸葛亮,代码终究是需要人去看的,通过与他人的交流,去寻求最佳实践,交流前提就是去分享自我,包括设计思想和实现路径
小到与一个人分享,也就是一对一code reivew,这样让review的开发人员了解代码的设计和实现,即能得到别人的指导,又能传递自我,并且能互为backup,方便后期维护,减少项目风险
大到与团队分享,产生技术氛围,让好的知识、设计在团队中分享,实现整体团队的成长和整体效益最大化
也鉴于要去把代码与人分享,就更容易让大家写出更具可读性的代码,提高可维护性,随便也让别人发现除功能逻辑外的一些技术逻辑:比如数据库连接是否忘记关闭,线程池是否正确使用等等,也加强了checklist的广度和深度
when
什么时候code review,大多数时候都是在上线前才做这件事,但理论最佳时间应该在提测前,以防测试完成后,又要对代码做变动
在实践时,可以拿出专门时间进行,以错开迭代发布的紧张期
除了上述的方法论,team leader还要在如何更好地code review,让团队更有意愿地参与上花心思,让团队成为一个学习型组织,有工程师文化的组织
简而言之,主体思想就是code reivew不应该把精力花在表面的code上,而是要关注设计思路,代码复杂度,工程熵增上,实现高内聚低耦合的大道上
References
[1]
《DDD总结》: http://www.zhuxingsheng.com/blog/ddd-opening-summary.html