CodeReview套路

CodeReview的好处

值得回味的是,大部分人都承认CodeReview是有好处的,之所以实际工作中不好落地或者效果不好,一个很重要的原因是因为CodeReview的效果短期很难看到,也很难量化衡量。我们会发现这样的例子在我们周围生活中也非常的多见,比如,我们都知道保持适量运动、健康饮食、规律作息都会使身体健康,但健康多少很难被量化,而且这些习惯需要长期坚持,才能潜移默化的见效。CodeReview也是如此,我们不能指望一次CodeReview就能改变什么,更多的是将它作为一种习惯,一种团队内部的固定流程,持之以恒的践行,才能起到效果,让我们的系统代码更加健康运行。那么拉长时间来看,CodeReview带给我们的好处有哪些,接下来就一一讲一下。

提升代码质量
「没有人Review过的代码,其水准就是写代码人的水准,而被一个团队Review过的代码,其水准将接近甚至超过整个团队的最高水准。」

这句话和我们经常说的“站在巨人的肩膀上”或者“众人拾柴火焰高”都是同样的道理。因为单个人可能在某些方便做的比较好,集大家之所长就能在各个方面都做的比较好。另外,随着CodeReview流程日常化,每个参与人的编码能力也会逐步提升,无限趋近于团队最高水准,因为在CodeReview的过程中,你可以看到别人做的好的地方,可以学习到经验,也可以看到别人做的不好的地方,吸取到教训。随着时间的流逝,逐渐积累参与者的能力。

提前发现问题
在没有CodeReview流程的时候,我们都是依赖于测试,坊间有一种说法叫做面向测试编程,代码的质量完全依赖测试用例进行驱动,更有甚者是依赖于功能上线后用户暴露问题,这种发现方式已经偏晚了,尤其是让用户暴露问题的时候已经是事故了。问题越晚暴露,风险也就越大。长久以往,我们会面临用户或者客户的严重不信任危机,这对一个以产品驱动为主的公司里面可以说是致命的。如果能在CodeReview这个阶段发现问题就能提前将各种风险扼杀在摇篮中。但是,CodeReview真的能提前发现问题吗?如果能的话,可以提前发现什么样的问题?

    CodeReview可以提前发现流程或者实现上的问题,尤其是在做业务需求的时候,不同工作经验的人对同一个需求的理解肯定会有差异,代码实现上也会有很大的差异,有时候一点点的理解偏差,导致出现那种失之毫厘谬以千里的后果,写代码的人也会因为处在自己的思维定式中发现不了问题,而别人可能因为经验丰富或者需求相关背景了解更多,就能很轻易发现问题。

像这种情况一般出现在团队新人身上,他们对已有系统和业务了解不多,实现需求时很容易出现问题,这时候如果有人帮忙指出就能避免更严重的后果,另外也有助于新人了解业务和融入团队。

在CodeReview过程中另外一种问题也比较容易发现,就是那些别人之前踩过的坑。像是诸如:Java中SimpleDateFormat的线程安全问题;Java日期 LocalDateTime 数据精度问题;一些开源框架中所使用的组件安全问题等等,这种例子就数不胜数了。所以团队组织内部维护一些文章或者教程之类的,也能帮助我们的代码质量提高,另外就是,我们可以在一些代码检查工具中维护自定义的检查策略,用来将这些坑自动检查出来,诸如CheckStyle,Sonar等。

当然CodeReview也不是万能的,也有很多问题发现不了的,比如一些边缘Case或者是代码计算结果的准确性,比如你代码实现了一个很复杂公式的计算,你总不能指望有人能通过看代码来发现问题吧。这些还是老老实实去做测试吧!

经验和知识的传递
工程师可以写出能运行的代码,但真正高级的工程师才能写出低bug、易扩展、易维护的高质量代码,更高级的工程师还能帮其他人成为一名合格的工程师。CodeReview也是高级工程师来帮助其他人最直接的方式。

在CodeReview中,你可以看到其他人写出来的优秀代码、优秀的设计,甚至和改动相关的业务背景知识……,Review越多,学到的也越多。另外,即便是哪些不太好的代码,经过别人的Review,必然会留下很多评论或者是改动建议,甚至是别人之前踩的坑,你都能看到,从这些内容上你也可以学到很多新的知识。CodeReview不仅仅是一个提升代码质量方式,它也可以肩负起知识积累和消息集散的功能。我们经常说的打造工程师文化的组织,CodeReview在这方面是能起到很大的正向作用的。

所以,Review别人代码时请毫不吝啬地留下你的建议吧,另外,CodeReview时也不要忘记关注下别人的建议,说不定可以学到新的东西。


CodeReview的套路

就像CodeReview不是银弹一样,我们也无法指出一套最佳实践用作我们的指导,但好在有很多可以参考的步骤。

建立良好的CodeReview标准
首先要明确,CodeReview是为了让代码越来越好的工作,所以CodeReview者需要对代码的整体质量负责。他们需要确保代码库一致、可维护。

其次,CodeReview者要再次明确CodeReview是为了让代码越来越好的工作,而不是追求一次完美,要知道没有完美无缺的代码,只有越来越好的代码。那么只要代码的变更对系统有明显的提升且正常工作,即便不完美,评审人员也应该倾向于通过这次变更。

最后,重要的事情说三遍,CodeReview者要明确CodeReview是为了让代码越来越好的工作,每次的评审都建议寻求是否有更好的想法,这部分可以在评论中不断的告诉开发,加入到TODO List中去,为以后的变更提供基础。这里有一点需要明确,我们的评论里面如果有带有教导开发者的内容的话,可以在前面加上“Nit:”,或者明确指出不需要在本次变更中解决。

了解变更的背景
CodeReview切忌直接看代码,那纯粹是浪费时间。CodeReview虽然是Review代码,但是首先评审人员应知道代码实现了什么样的功能,是在什么样的背景下去做的变动,清楚前因后果之后,才能代入到开发者的角色,你才能更好的去Review别人的代码、去发现别人的问题。

通盘考虑
了解背景之后,有一个大概的实现思路,也有个流程主线。这个时候如果和开发者的思路相同,那是最好不过的,只要顺着共同的思路去帮忙Review整个流程是否正确即可。而另一种情况就是思路不同,这是就要看代码去了解写作者的思路,然后确认是谁的思路有问题,或者是谁的思路更好,然后同开发者一起将这个流程优化到更优。

逐层细化
确定完整个流程之后,就可以逐步深入到代码细节中了,细节可以Review的地方就很多了,下面会详细讲到。

CodeReview的内容

整体设计
把握住变更中的整体设计。变更中各个部分的代码交互是否正常?是否和系统中其他部分交互正常?现在是否是添加整个功能的恰当时间?

功能性
开发者想在这个变更中实现什么?打算使用该变更为用户带来什么好处?(这里”用户“是指受变更影响到的实际用户,和将来会使用到这些代码的开发者)

这里评审人员希望提交评审的代码是经过充分测试的,以确保代码在CodeReview期间正常运行。

评审人员需要验证变更,尤其是在有面向用户的影响时,评审人员应该仔细检查整个变更。有时候单纯看代码很难理解这个变更如何影响到用户,这种情况下可以让开发者为你提供一个功能性的demo。

另一个在CodeReview时需要特别关注的点是是否有 并发编程,并发理论上可能会导致死锁或资源争抢,这种问题在代码运行时很难被检测出来,所以需要有人(开发者和评审人员)仔细考虑整个逻辑,以确保不会引入线程安全的问题。(所以除了死锁和资源争抢之外,增加CodeReview复杂度也可以成为拒绝使用多线程模型的一个理由。)

复杂性
变更是否比预期的更复杂?检测变更的每个级别,是否个别行太复杂?功能是否太复杂?类是否太复杂?”复杂“意味着代码阅读者很难快速理解代码,也可意味着开发者尝试调用或修改此代码时可能会引入bug。

一个典型的复杂性问题就是 过度设计,当开发者让代码变得更通用,或者增加了许多当前不需要的功能时,评审人员应该额外注意是否过度设计。鼓励开发者解决现在遇到的问题,而不是揣测未来可能遇到的问题。未来的问题应当在遇到时解决,到那个时候你才能看到问题本质和实际需求。这里要注意的是保留扩展的可能和过渡设计的差别。

测试
如前面所述,开发人员应当进行单元测试、集成测试或端到端测试。一般来说,变更中应该包含测试,除非这个变更只是为了处理紧急情况。

确保变更中的测试是正确、合理和有用的。因为测试本身无法测试自己,而且我们很少会为测试编写测试,所以必须确保测试是有效的。

如果代码出了问题,测试会失败吗?如果代码发生改动,它们会误报吗?每一个测试都有断言吗?是否按照不同的测试方法对测试进行分类?

记住,不要以为测试不是CodeReview中的一部分就不关注其复杂度。


命名
开发人员是否使用了良好的命名方式?好的命名要能够充分表达一个项(变量、类名等)是什么或者用来做什么,但又不至于让人难以阅读。可以参考如下的建议

l 命名的关键是能准确达意。对于不同作用域的命名,我们可以适当地选择不同的长度。

l 我们可以借助类的信息来简化属性、函数的命名,利用函数的信息来简化函数参数的命名。

l 命名要可读、可搜索。不要使用生僻的、不好读的英文单词来命名。命名要符合项目的统一规范,也不要用些反直觉的命名。

l 接口有两种命名方式:一种是在接口中带前缀“I”;另一种是在接口的实现类中带后缀“Impl”。对于抽象类的命名,也有两种方式,一种是带上前缀“Abstract”,一种是不带前缀。这两种命名方式都可以,关键是要在项目中统一。

注释
开发者有没有写出清晰易懂的注释?所有的注释都是必要的吗?对于注释,可以参考如下建议:

l 注释的内容主要包含这样三个方面:做什么、为什么、怎么做。对于一些复杂的类和接口,我们可能还需要写明“如何用”。

l 类和函数一定要写注释,而且要写得尽可能全面详细。函数内部的注释要相对少一些,一般都是靠好的命名、提炼函数、解释性变量、总结性注释来提高代码可读性。

l 变更中附带的其他注释也很重要,比如列出一个可以移除的待办事项,或者一个不要做出代码变更的建议,等等。

l 注意,注释不同于类、模块或函数文档,文档是为了说明代码片段如何使用和使用时代码的行为。

代码风格
团队内部应该拥有明确的风格,对于上述的注释和这里面提到的代码风格都应该收归统一,确保所有人都写出风格一致的内容。这部分才是实际落地中最难的部分,一定要在CodeReview 督促执行。持之以恒才能对代码质量起到立竿见影的效果。这里有些建议可以参考:

l 函数、类多大才合适?函数的代码行数不要超过一屏幕的大小,比如 50 行。类的大小限制比较难确定。

l 一行代码多长最合适?最好不要超过 IDE 的显示宽度。当然,也不能太小,否则会导致很多稍微长点的语句被折成两行,也会影响到代码的整洁,不利于阅读。
l 善用空行分割单元块。对于比较长的函数,为了让逻辑更加清晰,可以使用空行来分割各个代码块。

l 四格缩进还是两格缩进?我个人比较推荐使用两格缩进,这样可以节省空间,尤其是在代码嵌套层次比较深的情况下。不管是用两格缩进还是四格缩进,一定不要用 tab 键缩进。

l 大括号是否要另起一行?将大括号放到跟上一条语句同一行,可以节省代码行数。但是将大括号另起新的一行的方式,左右括号可以垂直对齐,哪些代码属于哪一个代码块,更加一目了然。

l 类中成员怎么排列?在多数的编程规范中,依赖类按照字母序从小到大排列。类中先写成员变量后写函数。成员变量之间或函数之间,先写静态成员变量或函数,后写普通变量或函数,并且按照作用域大小依次排列。

l Alibaba的JAVA开发规范、Google Style Guides都为风格和规范提供了指导,团队内部可以根据实际情况来进行采用。

l 如果想对风格规范中没有提及的风格做出改进,可以在注释前面加上“Nit:”,让开发人员知道这是一个你认为可以改进的地方,但不是强制性的。但请不要只是基于个人偏好来阻止变更的提交。

文档
如果变更改变了用户构建、测试、交互或者发布代码相关的逻辑,检测是否也更新了相关文档,包括README, API Swagger页面,以及任何相关文档。如果开发者删除或者弃用某些代码,考虑是否也应该删除相关文档。如果文档有缺失,让开发者补充。

编码技巧
关于编程技巧的部分,这里更多的是靠经验的丰富对于需求的理解,碍于篇幅有限,无法细讲,开发人员需要多读优秀代码,从尝试模仿开始,慢慢过渡到在自己写的代码里面加入设计模式,最终成为经验丰富的开发者。给出一些建议如下:

l 将复杂的逻辑提炼拆分成函数和类。

l 通过拆分成多个函数或将参数封装为对象的方式,来处理参数过多的情况。

l 函数中不要使用参数来做代码执行逻辑的控制。函数设计要职责单一。

l 移除过深的嵌套层次,方法包括:去掉多余的 if 或 else 语句,使用 continue、break、return 关键字提前退出嵌套,调整执行顺序来减少嵌套,将部分嵌套逻辑抽象成函数。

l 用字面常量取代魔法数。

l 用解释性变量来解释复杂表达式,以此提高代码可读性。

上下文
一定要去看看改动的上下文代码,这对CodeReview非常帮助,因为通常CodeReview中一些工具也好,开发者也好,通常会关注在改动部分上下几行代码,但有时评审人员必须查看整个文件确保这次变更在整个链路中可以正常运行。

CodeReview时考虑到整个系统的上下文也很重要,这次改动提升了系统健康度?或者增加了系统复杂性?少了测试用例?…… 不要通过哪些会损害系统健康的代码。 很多系统变复杂都是因为一点一点的小改动日积月累造成的,所以千万不要放进来任何增加复杂性的改动。

代码亮点
对于变更中的亮点,一定要开发者他们做的很棒,尤其是当他们用某种很精巧的方式解决你评论中提到的问题时更应不吝赞美。 虽然CodeReview主要是关注于错误,但也应该为开发者展现出好的一面点赞。在指导他人的时候,有时候告诉开发者正确的做法比告诉他们错误的做法更有价值。

尺度问题
就像上面曾经说过的,CodeReview的目标是让代码库越来越健康,绝不应该要求开发者打磨好变更中的每个细节才予以通过,相反,评审人员应该权衡项目进度和他们给出建议的重要性,适当放宽要求。评审人员应该追求持续提高 ,而不是追求完美。那些可以提升整个系统可维护性、可读性和可以理解性的变更不应该因为代码不够完美而被推迟几天甚至几周。对于一些有可能使代码更好的建议,可以在评论中提出,敦促开发者加入TODO LIST,在将来的版本中持续改进。

解决冲突
如果代码评审中有任何冲突,开发者和评审人员都应该首先根据规范文档的内容,尝试达成一致意见。

当很难达成一致时,开发者和评审人员不应该在代码评审评论里解决冲突,而是应该召开面对面会议或者找个权威的人来协商。(如果你在评论里协商,确保在评论里记录了讨论结果,以便日后其他人翻阅。)

如果这样都解决不了问题,那解决问题的方式就应该升级了。通常的方式是拉着团队一起讨论、让团队主管来权衡、参考代码维护者的意见,或者让管理层来决定。不要因为开发者和评审人员不能达成一致而阻碍变更进行下去。

结尾

好了,到这里基本就是CodeReview的所有内容了,最后再澄清一下对于CodeReview的误区:

CodeReview并非无用或者浪费时间
这是普遍的误区,只要坚持进行CodeReview,我们的代码就会越来越健康。

CodeReview并非银弹
做了CodeReview,并不意味的万事大吉,CodeReview是一个长期的过程,无法指望它能代替测试环节。

CodeReview并非立杆见效
CodeReview是一个长期的过程,需要坚持不懈的落地执行,才能看到最终效果。

评论
添加红包

请填写红包祝福语或标题

红包个数最小为10个

红包金额最低5元

当前余额3.43前往充值 >
需支付:10.00
成就一亿技术人!
领取后你会自动成为博主和红包主的粉丝 规则
hope_wisdom
发出的红包
实付
使用余额支付
点击重新获取
扫码支付
钱包余额 0

抵扣说明:

1.余额是钱包充值的虚拟货币,按照1:1的比例进行支付金额的抵扣。
2.余额无法直接购买下载,可以购买VIP、付费专栏及课程。

余额充值