淘宝App出那么大bug,是怎么做Code Review的?一位阿里技术专家,说出了真相!
技术领导力
作者简介:孤尽,阿里技术专家,曾出版畅销书《阿里巴巴Java开发手册》、《码出高效》,擅长JAVA编程、工程效能提升。
几天前,淘宝App“内测版本弹屏P0级事故”,把淘宝IOS团队推到了舆论的风口浪尖,许多程序员朋友纷纷质疑,你们不做CodeReview的吗?
说实在话,小编也很好奇,阿里技术团队内部究竟是如何做代码审查的?是不是跟我们一样流于形式?本文作者,孤尽,介绍了阿里巴巴技术团队的代码审查实践,一起来学习吧。
正文
最近 CodeReview(以下简称CR )心态相当的平和,代码是一个讲道理的东西,是就是,否就否。在 CR 时,沟通特别轻松,问题讨论也特别聚焦,因为它是量化和定向的。CR 的过程不是恃强凌弱,也不是一言堂,大家看着代码,当作是一种灵魂的交流,那么每一次的 CR 也是同事间提升和谐度的一种方式。优良的 CR 传统可以体现团队温度,体现高年级同学传帮带的技术文化。平时,大家抬头看 PRD ,低头写代码,很少有时间静心气闲地交流一下业务流程、业务逻辑、业务未来扩展,在 CR 时,往往可以反复被讨论到。一个人的能力不是体现在解决了问题上,也不是发现了问题,而是利用某种手段预知问题并解决问题。曾经有段代码,我觉得取反逻辑生涩难懂,反复修改之后,发现写代码的小伙伴是错误领会了业务意图。
提升技术质量、促进人才成长、培养技术情怀这些口号我们今天先放一边,聊聊最近CR的切身体会。CR 不是互相看天书,而是产生天天看书的感觉,每一段写得好,写得不好的代码都是一本书,好的代码希望见贤思齐,差的代码希望见不贤而内自省也。总之, CR 是一种修行,也是一种自我积累,苦涩的是看到惨不忍堵的代码,心里说:我去!有意思的是看到优雅的代码,心里也说:我去!
业务跑得这么快,没时间 Code Review
这是一个很大的谎言,不要为自己的丑代码找华丽的借口,没有时间好好 CR ,总有时间焦头烂额地处理故障和投诉。
时间老人是公平的,我一直认为某个同学在工位上噼里啪啦打字,就是说明他干活快,通过团队打字比赛,发现其中 20% 在按 BACKSPACE 键。业务跑得快,代码写得快,可能写的是一堆没有营养甚至是有毒的代码。我们需要追求的是 CR 的效能,而不是逃避 CR 。CR 是一种修行,对于双方都是一样的收获,因为如果想象成一个摊派任务,抵触情绪总会油然而生。业务跑得快,也得两腿是健康的, CR 就是保证业务持续跑的快的一个小医生,不正常的业务节奏对公司的中长远发展肯定是弊大于利。
代码是讲道理的
我认为靠烧香来保佑代码不出问题时,保平安往往也是暂时的。优秀的代码,就是在小流量、单线程没有问题,在高流量、高并发时还是没有问题,你的限流,你的容灾,你的降级各种导弹防御系统一样自动打开并正确地发挥价值。很多人的思维觉得,代码只要在场景和逻辑上没有问题就行,那是因为夜路走得不够多,还没有碰到鬼。代码是讲道理的,就像有一个同学说>=比>更加慢,那只是我们的潜意识猜测,经过深达编译层的分析,发现两个指令几乎是完全一样。其实凭我们的想象,那也是一个位运算级别的操作,从左向右比,如果一处有 1 ,另一个没有 1 ,那么前者一定是更大。没有无缘无故的爱,没有无缘无故的恨,一切的故障总是代码的字里行间。我们需要做的,就是读懂她,用好她,写好她。如果代码任性闯祸,只能是我们不懂代码的心思。
每一行代码的存在是有意义的
更加严格地说,每一个字符的存在都应该是有意义的。如果某行代码的存在完全是可有可无的,这个时候,我们考虑过 JVM 的感受吗?凭白无故地要编译这些字节码,然后栈进栈出的忙活一阵子,然后告诉它,你的劳动是没有任何价值的。比如,Boolean assetFlag = Boolean.true ; 这里都已经明确地给给出来显示的初始值,可是在调用端,居然还有这样的判断:if ( assetFlag != null && assetFlag == true) {...},什么情况下为 null 值啊?另外参数在框架里已经做了值的判断,那么下边又是 n 行,对所有参数重新判断一遍,是对我们的代码有多少不自信,还是对框架不自信?每一行的代码,相当于生命,它的存在一定是有意义的,一定是能够被执行到并且能够为实际的业务负责的。
我们比拼的不是代码行数
在 Code Review 过程中,发现有些方法,重复用到一段逻辑,这段逻辑如果不抽取出来成为一个方法,未来的修改就成了一个必须多点全部修改的大坑,稍有不慎,容易遗漏。重复代码在提交行数上,似乎挺壮观的。如果在同样的效果上, 3 行代码能够实现功能的价值,就不应该用 4 行来实现。我们经常说晒出代码行数,并非是单纯地鼓励代码行数多,而是提倡大家去写代码,写优质的代码,优质的代码一定是少即是多的原则。代码的实现,不要像鲁迅先生说的一样:懒婆娘的裹脚布又臭又长。
用户视角的成功与失败
在交付时,调用服务失败,然后返回前台一个空列表,那么前端业务的展示是后台数据正常,这个人不拥有数据列表,这明明是对数据的一种曲解。所以,后台调用服务失败,就应该明确告诉前台,服务出错了,这个用户有没有数据。系统出错的信息给用户看,合适吗?不合适。前后端的用户交界面上,往往飞着两类信息:错误码、错误信息。这样够了吗?用户提示需要额外地再给出来,往往根据不同的错误码,有不同的用户提示,可能是一个多对多的关系。多个错误码,提示给用户的信息:请输入必填项。多个用户信息,可能也对应一个错误码。一般来说后台承包这三者的联动关系, json 串推送给前端时,前端拿来主义即可。
有重复使用的量一定要找个地方集中隔离
不管是变量,还是常量,工具类,如果是多个地方同时用到,那么如果硬编码在代码或者沉淀在包里,未来一定是一个灾难。比如,一个组装 SQL 语句的代码,到处都是 "from" "where" "limit" ,都是这类语句直接写死在代码中,注意问题来了,这些单词前后都需要加空格。有时候在复制粘粘时,发现少了一个空格,出现的问题,往往是致命的。再比如,一个互相约定的分隔符 “###” ,定义在本类中 private String ,这明显是两个共同遵守的常量,单独定义的结果就是容易造成不匹配。隔离的目的是复用它,保护程序地正常运行,易于维护。
单测没必要代码 Code Review
单测有时候感觉像是阑尾,有或没有感觉都是无关紧急,这是错误的观点。单测感觉就是一个任务。你写单测了吗?写了。单测是否需要 MOCK ,是否进行边界值测试,是否用例覆盖到业务场景,这都也是 CR 的一部分。单测写得好, BUG 肯定少。
需要调试来查找错误时,往往是一种对异常处理机制的侮辱
良好的日志和异常机制,是不应该出现调试的。打日志和抛异常,一定要把上下文给出来,否则,等于在毁灭命案现场,把后边处理问题的人,往歪路上带。别人传一个参数进来,发现是 null ,立马抛出来一个参数异常提示,然后也不返回哪一个参数是 null ,这在调用参数很多的情况下,简直就是字谜游戏一样。到底是抛异常,还是抛错误码?我不管抛什么,反正错了什么东西,都应该透明出来。到底是抛受检异常,还是非受检异常,我只想说,没有充足的理由,不要乱抛受检异常。异常抛出时,一定要自己消化干净,告诉别人说我的方法签名抛的是 AbcException ,实际运行中,代码某个地方直接抛出 EfgException ,这也是不负责任的。
多个 return 的语句,概率高的一定先进行判定
if(condition1) return; if(condition2) return; if(condition3) return ; 那么需要评估一下 condition1/2/3 出现概率的大小,概念大的在最前边,尽可能快地进行 return ,不需要进行后续无谓的匹配。不要总觉得计算机跑得快,不差这点蝇头小利的,这种思维,和《南辕北辙》里的寓义一样的吗?
吝啬空行
感觉空行是廉价的,到处乱扔是一种;另一种是感觉空行是昂贵的,舍不得用,这种情况更多见。50 行代码没有一个空行,就像英语 50 句话,没有任何标点符号一样。既然标点符号起到隔断和语义区分作用,我们的空行不是同一个道理吗?在以下情形:1、在方法的 return、break、continue、这样断开性语句后必须是空行。2、在不同语义块之间。3、循环之前和之后一般有空行。另外,方法和类定义下方就不需要空行了吧。
命名太随意
代码有两件事情比较头疼:命名和循环。人如其名,如果不是它干的活,名字却是一副道貌岸然,太容易把人带偏了,一个中国人如果取名叫赵C,一个女孩子如果取名叫石敢当,第一印象生生地给扭曲了。英语不好的同学,要么用错英文单词,要么翻词典,整出一个专八的词汇,任何人都不认得这个单词,在 CR 时,还需要打开在线翻译时的命名,绝对不是好命名。当然如果在线翻译都翻不出来的时候,那更头疼。如果表意错误,那更要命。
注释是电影的旁白
电影的旁白:1)信息量大。2)适时出现。就像 star war 里,开始的一段一样,如果不交代那些背景,可能进入正片是一脸懵逼的。在代码上不需要写正确的废话,名字取得好,自然是自解释的。在嵌套循环中,或者在复杂条件分支中,往往是需要讲明白的。
另外,添加业务背景信息,以及执行频率,执行条件,甚至维护者注意点,都是注释的重要理由。识别到哪里要写注释,也是一个对业务的阅读能力,而不是代码阅读能力。
满天飞的函数式编程好吗?
答案是:不好。如果一个 stream 后边的调用超过 5 个,我觉得你是为了炫耀,因为别人不敢改这段代码,体现出来你的不可替代性。这种 10 行都是函数式编程的方式,就像让人在水里憋气超过 10 分钟不能换气一样难受,有点缺氧的感觉。如下图,反对这种直接 return 一个长链路的处理结果:
孤尽说:“亲力亲为写代码,写稳定和优质的代码,才能激发强大生产力”。我们希望通过“文化月”的形式,传播技术文化,向代码致敬。
编程是一门艺术,优秀的代码看起来是优雅的,希望在传承代码文化的同时能让更多开发者收益,共同用优雅的代码谱写社会的未来。
以下文章来源于阿里巴巴中间件 ,作者孤尽