Code Review 是一场苦涩但有意思的修行

分布式实验室 · 公众号 · 后端 · 2021-01-05 07:45

正文

前言

最近 CodeReview（以下简称CR ）心态相当的平和，代码是一个讲道理的东西，是就是，否就否。在 CR 时，沟通特别轻松，问题讨论也特别聚焦，因为它是量化和定向的。CR 的过程不是恃强凌弱，也不是一言堂，大家看着代码，当作是一种灵魂的交流，那么每一次的 CR 也是同事间提升和谐度的一种方式。优良的 CR 传统可以体现团队温度，体现高年级同学传帮带的技术文化。平时，大家抬头看 PRD ，低头写代码，很少有时间静心气闲地交流一下业务流程、业务逻辑、业务未来扩展，在 CR 时，往往可以反复被讨论到。一个人的能力不是体现在解决了问题上，也不是发现了问题，而是利用某种手段预知问题并解决问题。曾经有段代码，我觉得取反逻辑生涩难懂，反复修改之后，发现写代码的小伙伴是错误领会了业务意图。

提升技术质量、促进人才成长、培养技术情怀这些口号我们今天先放一边，聊聊最近CR的切身体会。CR 不是互相看天书，而是产生天天看书的感觉，每一段写得好，写得不好的代码都是一本书，好的代码希望见贤思齐，差的代码希望见不贤而内自省也。总之， CR 是一种修行，也是一种自我积累，苦涩的是看到惨不忍堵的代码，心里说：我去！有意思的是看到优雅的代码，心里也说：我去！

业务跑得这么快，没时间 Code Review

这是一个很大的谎言，不要为自己的丑代码找华丽的借口，没有时间好好 CR ，总有时间焦头烂额地处理故障和投诉。

Photo @ Djim Loic

时间老人是公平的，我一直认为某个同学在工位上噼里啪啦打字，就是说明他干活快，通过团队打字比赛，发现其中 20% 在按 BACKSPACE 键。业务跑得快，代码写得快，可能写的是一堆没有营养甚至是有毒的代码。我们需要追求的是 CR 的效能，而不是逃避 CR 。CR 是一种修行，对于双方都是一样的收获，因为如果想象成一个摊派任务，抵触情绪总会油然而生。业务跑得快，也得两腿是健康的， CR 就是保证业务持续跑的快的一个小医生，不正常的业务节奏对公司的中长远发展肯定是弊大于利。

代码是讲道理的

我认为靠烧香来保佑代码不出问题时，保平安往往也是暂时的。优秀的代码，就是在小流量、单线程没有问题，在高流量、高并发时还是没有问题，你的限流，你的容灾，你的降级各种导弹防御系统一样自动打开并正确地发挥价值。很多人的思维觉得，代码只要在场景和逻辑上没有问题就行，那是因为夜路走得不够多，还没有碰到鬼。代码是讲道理的，就像有一个同学说>=比>更加慢，那只是我们的潜意识猜测，经过深达编译层的分析，发现两个指令几乎是完全一样。其实凭我们的想象，那也是一个位运算级别的操作，从左向右比，如果一处有 1，另一个没有 1，那么前者一定是更大。没有无缘无故的爱，没有无缘无故的恨，一切的故障总是代码的字里行间。我们需要做的，就是读懂她，用好她，写好她。如果代码任性闯祸，只能是我们不懂代码的心思。

Photo @ Hitesh Choudhary

每一行代码的存在是有意义的

更加严格地说，每一个字符的存在都应该是有意义的。如果某行代码的存在完全是可有可无的，这个时候，我们考虑过 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 句话，没有任何标点符号一样。既然标点符号起到隔断和语义区分作用，我们的空行不是同一个道理吗？在以下情形：

在方法的 return、break、continue、这样断开性语句后必须是空行。
在不同语义块之间。
循环之前和之后一般有空行。另外，方法和类定义下方就不需要空行了吧。

命名太随意

代码有两件事情比较头疼：命名和循环。人如其名，如果不是它干的活，名字却是一副道貌岸然，太容易把人带偏了，一个中国人如果取名叫赵C，一个女孩子如果取名叫石敢当，第一印象生生地给扭曲了。英语不好的同学，要么用错英文单词，要么翻词典，整出一个专八的词汇，任何人都不认得这个单词，在 CR 时，还需要打开在线翻译时的命名，绝对不是好命名。当然如果在线翻译都翻不出来的时候，那更头疼。如果表意错误，那更要命。

Photo @ Jon Tyson

Code Review 是一场苦涩但有意思的修行

正文

请到「今天看啥」查看全文