【老万】我看代码审查（二）：修炼的要点
题图来自
衡量程序员能力的维度有很多，包括设计、编程、领导、沟通、写作、颜值等等。其中编程能力做为程序员的安身立命之本，我觉得是最重要的一个维度。代码写得好，要饭要到老？那是写不来代码的同学黑我们的！代码写得好，不一定能成为大牛，但是我没有见过哪个大牛程序员是不屑于写好代码的。有人说，设计难道不是更重要吗？是的，设计能力很重要，但说到底这是是编程多了建立起来的一种感觉。世界上本来并没有路，代码写多了，就有了套路。而设计，就是知道什么情况下该用什么套路。所以，归根结底代码要写得好。
程序员在编程能力的进化途径上，我认为要经历四个阶段：刚出学校的初级程序员，需要解决的是温饱，啊不，是代码的正确性。就是说让代码干咱们想让它干的事儿，不要干咱们不想让它干的事儿。至于干得好不好，跑得有多快，长得好不好看（我是说代码）......，他们一时还顾不上。中级的程序员，要追求系统的效率和稳定性。代码不光要干正确的事，还要吃得足够少（少占内存、CPU、文件等资源），跑得足够快，在满负荷的情况下不要崩溃。高级的程序员，在此基础上还要保证代码的可维护性和可扩展性，降低开发成本，提升系统对需求改变的应对能力。这样，有一天他以身殉职倒在生产线上的时候，同事们在洒下鳄鱼的眼泪的同时，还可以踏着他的遗体继续前进。高级之后，要成为大牛，必须要有卓越的眼光，知道什么问题是最值得他花时间去解决的，写哪些代码能产生最大的绩效。这里说的绩效，不一定是炫酷的新功能，也包括提高系统性能和代码质量的优化、重构。我发现，处于入门境界的程序员，一般容易忽视重构的重要性，也不知道怎样重构才好。境界提升之后，又要警惕走入另一个误区，花太多时间重构系统。如何最搞笑，咳，最高效地用好自己的时间，如何时候都是一个重要的技能。
不明就里的群众可能会以为代码审查是一种高级技师对低级技师的培训。其实，代码审查是一个相互学习的过程。这些年我通过审查别人的代码学到了不少先进经验，也把我的浅见传播到了千家万户（就是黄老千家和我家）。我们程序员只有工分（就是干一个小时挣多少钱）不同，在审查代码的时候并没有高低贵贱之分。像我，就审查过我歌资深院士 Jeff Dean 的代码，值得炫耀。吗？我有拿出来到处说吗？
审查人的职责之一，是把代码的作者往高层次带。好的审查人，不光要指出代码中的不足，还要用最能被作者接受的方式指出，帮助作者更快地进步，把团队建好。和写代码一样，审查代码也是一门手艺，需要不断的学习、用心，才能精进。我审查代码十多年，不敢说自己审得多好，起码踩过不少坑，有一些经验。把这些经验写下来，能让大家少踩几个坑，我就欣慰了。
先讲几条代码作者需要注意的事项：
不要在设计取得共识之前请求代码审查。设计和实现，是两个层次的东西，前者是画蓝图，后者是施工。如果一个系统的蓝图画得毛扎扎或者是想到哪造到哪，造出来的东西多半是老虎的头配猪的脖子加鸡的翅膀和乌龟的壳，怎么看怎么不是个玩意儿。所以，大多数情况下要早早审查系统设计，然后才开始编码。问题没有想清楚就动手？可以，那是在摸索阶段。设计没搞好就一鼓作气做出一个系统或者模块，远看也能跑，近看是玩具，徒靡弹药止增笑耳。有的人急性子，恨不得审查人一言不发盖个戳就好，这样可以马上转战下一个功能，把悲伤留给同事，升职归于自己。结果，他们做了很多无用功。废半天劲造好的豪宅，被发现千疮百孔，只得推倒重来。早年我做 Google Test 的时候，老板给我招了个新人帮忙。这位同学初来乍到，想做个大功能扬名立万，哗一下给我送了一个大的CL（🆑，即 change list，指逻辑上相关联的一组改动。在版本管理系统中提交代码以CL为单位。）。我一看，不好，好多设计上的错误呢。于是我们就展开了讨论。这一开始就把不着边儿了，来回有一两百封邮件才搞定（此处并无使用夸张修辞手法）。现在想来，我也犯了一个错误：邮件两三次之后发现进展不快，就应该马上停下来改成开会，在一个高带宽的信道把认识统一了，再谈执行。
先做好自己的功课，不要把一个半成品甩给别人审查，让别人当你的拼写检查器。这是对对方时间的尊重。唯一例外的情况是对方案的大方向还不确定时，可以先发一个征求意见稿，但这种情况一定要先讲清楚，免得对方在细节上浪费太多时间。一般来说，我在把一个CL送出去之前会先做以下这些事情：
确保一个CL不要太大。动不动就几千行，让人望而生畏。一般来说，每个CL只做一件事，这样看起来省力而且出了问题回滚方便。我有个经验公式：一个CL的复杂度和它大小的1.5次方成正比。这个1.5是怎么来的？首先，如果一个CL中每行改动都是独立无关的话，那么每一行单独审查就好了，不必考虑它们之间的依赖性和交互作用。这种情况下审查的工作量是线性的。要是全部改动都是息息相关的，我们就不能把它们孤立起来看，而是每一行都要考虑它如何会和其它行发生联系，在一起会不会起化学反应。这样审查的工作量就大大提升了，是平方关系。考虑到实际情况介于两者之间，就算1.5次方好了。从这个假设，我有一个推论：把一个大的CL拆成两个CL，审查起来更省力。有人说你这不是扯吗，改成小包装，总量并没有减小，工作量怎么可能下降？是这样的，一个CL不是随便拆开的，而是要有一定的逻辑性，保证拆分后的每个CL都是一个逻辑自洽的单位。拆分人把这个分解的工作做了，审查人就省事了，代码容易看懂，bug 也容易抓到。
确保编译通过，测试通过。这点看起来很蠢吧，可是我还是看见有人会送编译不过的代码给别人审查。
符合公司的编码规范和格式。有人说我司没有这些劳什子规定，那是不是就不用管这些了？雅蠛蝶！代码库没有一个统一的风格，工作起来是极辛苦的，因为要不停地花费能量在猜测作者用意上面。如果贵司还没有认识到风格的重要性，正是你力挽狂澜改变公司文化的好机会，还愣着干嘛，建立一个规范，你就是工程副总裁了。
注释清晰易懂有条理，读之如坐春风，尤其不能有自己能发现的拼写错误和语法错误。做到这点并不难，只要先去读一个博士学位就好了。等你熟读百篇 paper，再被导师和审稿人虐五年，自然可以不分时间场合引言结语张嘴就来而且严丝合缝。没时间读博士？那么写个博客公众号也是极好的。看到好的段子，咳，文章，用心想一想好在何处，你从中能学习到什么新技能，再在自己写作时强行使用几次，水平肯定进步。总之要用心。
每个CL都有个描述区，用来写关于这个CL的总注释。如果说一个CL是一篇论文，这个描述就是论文摘要。有经验的工程师会在这里简明扼要地写清楚这个CL要解决的问题和方案，方便日后检索。可是有很多同学不把这个CL描述当回事，千篇一律地写个“修一个 bug”或者是“改进系统性能”之类。这样一来增加审查的难度，二来以后系统出问题找原因的时候也会多好多事。
如果这个CL很复杂，两段话说不清楚要干啥，那就另外写一个设计文档，在CL描述里指向这个文档。这不光帮助审查人理解，也让自己理清思路。很多时候，在试图把一件事表述清楚的过程中就会暴露出很多问题。能把一个东西写清楚，给别人讲懂了，才是自己真正懂了。
在代码审查过程中，要举一反三：如果别人给你指出一个错误，那你就主动把这个CL中类似的错误都改了。不要让别人像挤牙膏一样挤你。大家都是成年人了，配合一下更愉快。
最后，对负责任的审查人心存感激。他们挑你的代码的毛病，不是要跟你做对，而是在帮助你提高。虽然被人指出不足会有些不爽，要把这看做学习的机会，而不是任性地骂娘。
做为审查人，也有一些需要留心的地方：
不要拖沓。要知道，你不OK，别人就没法提交代码。如果你实在是忙不能审查一个CL，早点告诉别人，他们可以马上另请高明。不要等到最后一刻才说对不起。我一般会在一个工作日之内回复一个CL。
有全局观。局部的最好往往不能带来全局的最优。如果一段代码只是一个试验性的原型而且时间很紧，那么精益求精的代价可能是影响了项目进度甚至引发连锁反应，让你的下游项目翻船。如果一个CL改动的是以后会被反复用到的基础模块，那就值得反复斟酌。
懂得妥协的艺术。写代码一大半是技术，一小半是艺术。说到艺术就没有一定之规。你的蜜糖，可能是他的砒霜。如果只是个人喜好，没必要强加于人。你可以解释一下你这么做的好处，如果对方愿意照办当然好，如果对方坚持己见，只要不是很难逆转的决定，就随他去吧。一来同事之间没必要为了艺术品味伤了和气；二来你也有可能是错的；三来一件事情的决定权最终还是应该交给这件事的具体负责人：将心比心，我们每个人都不喜欢当被别人牵线的木偶吧；四来很多时候人不撞南墙是不懂得回头的，你得让他为自己的错误付出代价，他才会真正体会你的良苦用心，就像在爱情中受伤的众生。像我一开始也是疾恶如丑，只认道理不讲人情，后来发现效果并不好。现在的我，百密一疏，在不关键的地方网开一面，这次我让你，下次你再听我的，皆大欢喜。
先扬后抑。批评别人的代码之前，先恭维几句。哪怕是一坨 shi，也可以夸它形态优雅，软硬适中。人是喜欢夸的动物，三天不夸，上房揭瓦。如果太直白，被批评的一方会觉得很受伤。所以提意见的时候保持谦恭的语气，与其说“你这段代码性能太差”，不如说“我觉得可以试试这种做法，性能可能会更高”。关于这一点，我做得也不够好，经常忙起来了就顾不上照顾同事的情绪，说得太直。让我们共同提高吧！
看人下菜碟。对高级职称的代码作者，要求可以相应的高一些，因为这是在他们的能力范围之内。对于初级同事，如果用同样的标准，那可能会彻底摧毁他的三观。而且也不可能一夜之间让一个二级程序员成长为宇宙间独一无二没有人有资格评判的大神。人的时间和精力都是有限的，提的意见太多他能记住的可能反而会更少。不如抓大放小，一次解决一个问题。只要事情是在向好的方向发展就OK。未竟的事业，写个TODO或者记录一个BUG下次再做就好了。
出来混总是要还的。技术债务积到一定程度会爆发。而且大家做同一个项目，在代码库里乱扔乱放增加他人工作难度，不是缺乏公德就是能力不够。所以我都尽力阻止一个CL引入新的技术债务，除非是在紧急情况下。
宣告式的（declarative）代码最美。软件开发，是一门关于抽象的学问。做房地产的都知道，房子最重要的三要素是：位置、位置、位置。做软件也有最重要的三点：抽象、抽象、抽象。什么是抽象？就是从具体、繁杂的操作中，抽取出高层次的概念，把实现的细节隐藏起来。我这个“抽象”的定义，本身可能过于抽象了。举个例子吧：计算机能直接理解的只有0和1组成的机器语言。这种语言，离机器近离业务远，用来直接写程序非常不现实，开发效率极低。把机器指令的编码方式抽走，改用英文指令名，就成了汇编语言，抽象程度比机器语言高一层，对程序员稍微友好一些。然而这还是太低级，离大多数业务隔了太多层，所以我们还要在上面包一层更抽象的高级语言，比如C++，比如Java。通用的高级语言运用起来方便多了，但相对于业务逻辑，经常还是不够抽象。比如“数一数在一个文件中‘伟大、光荣、正确’出现的次数”这个问题，用C++实现起来要涉及到相当多的细节，包括文件的打开和关闭，内存缓冲区的分配，字符串的匹配，计数器的累加，等等等等。如果所有逻辑都在一个函数内实现，就会很复杂（参见前面的万氏1.5次方复杂度定理），读起来不能快速 get 到要点。如果把一个个步骤包成小函数，取上含意清楚的名字，再在主函数中调用，主函数读起来就会很贴近业务逻辑，也就是说抽象程度更高了。代码容易读了，就不容易隐藏 bug。我在读博士时的最大收获，就是提高了对代码的审美力，对如何抽象有了更好的把握。简单地说，最好的代码应该是“宣告式的”（declarative），就是说它专注于告诉你它要干啥（what?），而不是具体如何干（how?）。见惯了好的，差的代码就能一眼识破了。
自动化测试。人非圣贤，除了某大神，谁都不能保证自己写的代码不会出错。即便今天没错，明天加一个新功能的时候也可能不小心搞坏已有的功能。自动化测试的用处，就是在一定程度上保证重构、改进系统的时候已经有的功能不会被破坏。有了这个保证，我们改造系统就可以更大胆，步子迈得大一点也不会扯着蛋。虽然写测试和维护测试花了一些时间，长期地看开发的进度是快了而不是慢了 ---- 当然，也有测试写得不靠谱得不偿失的情况，所以前提是测试要得法。写好测试也是一种高端的技能，需要用心学习提高，那里的山路十八弯，那里的水路九连环，这里就不展开说了，以后有机会再细聊。
自上而下。一个CL可能有很多层次上的问题：设计问题、正确性问题、效率问题、可读性问题......如果设计要改，很多代码可能都要推倒重写，这时候死抠这些代码的风格并无教育意义之外的意义，白白增加了工作量。我一般会先看有没有重大设计问题，如果没有，再看 class 和函数的合约（就是使用者和实现者之间的约定）是否合理（如果是 C++ 程序，意味着先看头文件）。这两样都没大问题了，再关注实现的细节问题。
下一次，我们会讲讲代码审查执行层面的一些事，欢迎关注。
往期文章推荐：
长按-识别二维码关注“老万故事会”公众号：