代码评审的三怕

简介: 代码评审的三怕

评审代码的流程


我自己总结的评审代码的流程,仅供参考:


1>确认代码的修改范围


2>梳理修改的部分的处理流程,之前没有流程图,需要先画流程图


3>进行逻辑正确性评审


4>对照代码规范和评审checklist进行规范性评审

 

三怕


一怕依赖版本更新


二怕检查出来的问题太多


三怕随手优化

 

依赖版本更新


依赖版本更新又分为两种,一种是二方库更新,另外一种是三方库更新。名词解释下:


一方库:本工程内部子项目模块依赖的库(jar包);


二方库:公司内部发布到中央仓库,可供公司内部其他应用依赖的库(jar包);


三方库:公司之外的开源库(jar包)。


二方库更新,一般来说,由于是公司内部的,有任何变更需要涉及方更新时都会通知,实际的改动量不会特别多。这时候升级前需要做版本比对,确认所有的修改处,尤其要注意:枚举值的顺序、方法的顺序是否有变换;方法是否有重载。


之前发生过由于在枚举值中间添加了新的枚举造成的故障。这和序列化和反序列化的实现有关。我之前的文章《JAVA数据处理的常用技术》里有提到protostuff这样的序列化方式之所以更省空间,是因为把原本的方法名比如thisIsMethod1做了编号,比如标记为1,代码这是第一个方法。而不是原本的名字。如果升级版本,比如thisIsMethod1和thisIsMethod2之间加了一个thisIsNewMethod。原本编号为:1,:2的实际上变成了:1,:3,:2。而序列化版本号没有同步更新,解析时会认为是:1,:2,:3,这时候会造成解析错误。如果觉得上面我说的不够清楚,可以这么来理解:有压缩功能的二进制算法都是有代价的,代价就是需要遵循一定规则,升级很可能会打破这个规则,造成错误。


三方库更新,通常由于漏洞会被要求升级,比如fastjson。对这类问题,首先明确这个jar引入解决的问题,针对这些功能做回归;第二,一个团队有多个应用的话,非核心应用先升级;第三,跟随策略,对于重要的服务,先等其他团队升级线上运行一段时间没有发现问题再升级;第四,尽量拉长灰度周期,避免小概率逻辑暂时没有验证到。


从上面二方库和三方库的升级方法来看,二者的升级都需要不小的工作量,作为代码评审的人要对结果负责。所以工作不仅是评审代码,还要确保上面需要的工作都做到位了。

 

检查出来的问题太多


很多编程人员可能都有这样的感受:在第一遍进行编码的时候,每一步会考虑的很周全细致。但是编写完成后一旦涉及到修改,可能就会改不全。最冤枉的是原来的代码本来问题也不大,只是风格不符合代码评审人员的习惯。结果评审人员非要编码人员改了,结果没改全出现了问题。虽然有一些手段进行规避,比如单元测试。但是修改的代价还是很高。我在进行代码评审的时候,第一件事是确认代码编写人员的修改范围,看看这个修改范围我之前有没有对逻辑做细致的梳理。如果没有梳理过,那我先梳理一遍,照着梳理的内容进行评审。代码风格的问题我会给出建议,但不会为此卡着不允许合入。毕竟为了避免程序出现逻辑问题,我每次把别人的代码打回去,下次提交我要照着梳理的逻辑整体全部重新评审一遍,避免疏漏。实际工作中概率统计:一个流程任务的执行。如果被检查出来的错误太多,说明有两点都做的不好:1>流程本身就有漏洞,没有标准化;2>执行者无脑或者缺脑的在执行。这样,检查人员要无限制的去做兜底。


对于写代码来说,如果代码写的漏洞百出,那说明方案评审阶段很多事情就没有理顺;写代码的人考虑问题也不够全面甚至没有仔细思考就动手写了。最可怕的是评审出来问题后,评审人让写代码者改什么,写代码者就是无脑照办。评审人很可能由于使用脑补细节有疏漏,造成问题。

 

随手优化


在《避免线上故障的10条建议》和《设计开发中要避免的两个坑和一种可借鉴的设计思想》里我都有详细说明过,随手优化可能会带来的灾难。但是最近写代码才发现这件事情我自己也没做好。


最近我提交了一次代码,被认真负责的同事加了评论打回了。第一条在一个地方给我打了个问号。这个地方我的修改是:在一个方法里,一个代码片段和下面的代码片段中间有2个空行。”因为我看着极为不爽就随手去掉了一个多余的空行。我看着打回的代码,本来是想解释一下,后来我仔细想了一下:


1>多一个空行完全不影响运行,也不会增加很多理解成本,本身必要性不大


2>我改了一个空行测试验证过吗?就算是理论上完全不影响生产,不经过测试的修改是不对的。


所以我默默的放下骄傲,还原了随手优化的代码。

 

后记


细节决定成败。刚毕业的时候,我们几个应届生一起做项目,做的都不是什么核心功能。就是C/S模式的客户端交互界面。我和旁边一个女孩相比。我做了7个界面,她做了1个。并且我的界面都比她的复杂。最后我做完了,她还没有做完,她延期了好久。最终领导说她做的好,我当时是非常不服气的。后来我想明白了:就像99%的时间会花在线上只有1%几率出现的问题一样。多囫囵吞枣的做事情并不多花多少时间,而想把质量提高一些,做的更好一些,却是更难的事。平时买东西也一样,质量高出一些,做工精巧一些,价钱多出好几个数量级;小时候考试也一样,从班里倒数第一进步到中等水平并不难,但是从第二名进步到第一名要的功夫就大多了;普通饭馆里凉菜十几元到几十元一盘,用了刀工的大厨做的那价钱会翻10倍甚至更高,一个道理。

相关文章
|
7月前
|
SQL 缓存 安全
一文浅谈CodeReview中的一些思考
CodeReview在日常的开发过程中越来越被重视,它在提高代码质量同时促进团队成员之间的知识共享和技能提升方面发挥了诸多作用,本文将主要围绕CodeReview展开,简单聊聊在CodeReview过程中的心得和思考。
89090 4
|
7月前
|
安全 Devops 测试技术
带你读《代码管理实践10讲》——六、代码评审到持续交付的最后一公里
带你读《代码管理实践10讲》——六、代码评审到持续交付的最后一公里
74 0
|
7月前
|
敏捷开发 安全 测试技术
带你读《代码管理实践10讲》——五、重评审还是轻评审,企业该如何选择代码评审模式?
带你读《代码管理实践10讲》——五、重评审还是轻评审,企业该如何选择代码评审模式?
125 0
|
7月前
|
Java Serverless Maven
云效代码管理问题之代码仓库配置需要评审如何解决
云效仓库是阿里云提供的代码托管和版本控制服务,支持Git等多种版本管理工具;本合集聚焦于云效仓库的使用技巧、团队协作流程以及常见问题解答,旨在帮助开发者更高效地进行代码管理和协作开发。
1853 0
|
SQL 安全 算法
codeReview
codeReview
130 0
|
SQL 缓存 NoSQL
代码评审的18个军规,收藏好!
大家好,我是田螺。 我们开发完需求,提测前,一般都需要代码评审。小伙伴们,你们知道代码评审,一般都有哪些军规嘛?今天田螺哥给你带来代码评审的18个军规。
208 0
|
安全 Java 测试技术
关于代码评审(CodeReview)那些不得不说的事儿
关于代码评审(CodeReview)那些不得不说的事儿
266 1
关于代码评审(CodeReview)那些不得不说的事儿
|
搜索推荐 测试技术 UED
需求评审那些事
需求评审那些事
|
测试技术
关于需求评审和讲解的一些思考
上图为[产品迭代开发协作流程],上次聊了一下关于 Code Review 的一些思考。 在上面的流程中,需求评审通过后,要产出最终的需求文档、原型等交付物,还要对本次需求封版;在测试阶段的补充需求的处理方式;上线后记录或反馈的问题列入下一版的迭代。 可是,流程中都是比较理想的状态,现实工作中并没有达到设想的目标。作为一个开发人员,分别站在产品经理的角度和开发人员的角度分析一下问题的可能原因。
|
开发工具 git 开发者
使用commitizen实现按团队规范提交代码
使用commitizen实现按团队规范提交代码
使用commitizen实现按团队规范提交代码