【老万】我看代码审查（一）：工具的变迁
先讲一个悲伤的故事。
读博的时候，我给一门编程课做过助教。有一天，我收到了这样一份作业：
#define ZERO 0
...
#define FIVE 5
#define SIX 6
…
if (size > threshold)
return SIX;
else if (x < ZERO)
return FIVE;
我问这位同学：你这是搞哪样？他说，老师在课堂上讲过，不要在程序里面直接用魔法数 (magic numbers，就是没头没脑冒出来的数值)。
显然，这位同学没有理解问题的真谛。不用魔法数，是因为读程序的人不知道它们背后的含义，会不知道作者到底想要干什么。从阿拉伯数字换成英文，换汤不换药，对这个问题没有任何帮助。正确的做法是给常数一个代表它意义的名字。这样，读程序的人就可以轻松搞懂它到底是干什么的。从上下文看意义很清楚的常数，不必再取名字。比如上面这段代码可以改写为：
const int invalid_input_error = 5;
const int capacity_exceeded_error = 6;
…
if (x > threshold)
return capacity_exceeded_error;
else if (x < 0)
return invalid_input_error;
试想，代码库中充满 #define FIVE 5 这样的代码，维护它的人一定是欲哭无泪。这就是为什么我们要有代码审查（code reivew）：代码在提交之前要经过同事评审，确定它符合一定的质量要求，正确性、效率、可维护性都要及格；否则就是埋下了一颗定时炸弹。
今天码工们对于代码审查这个制度应该都不陌生了：在绝大多数公司这都是一条起码要求。然而，在二十年前这还是一个新鲜概念。也许是我孤陋寡闻，在1998年之前，无论是在学校做论文还是打工，我和周围的同学同事从来都是文责自负，写什么都是自己说了算，代码库里充斥了各种奇葩段落。我在出国前甚至连版本控制（version control，就是把一个代码库的所有版本放在一个数据库当中，可以方便比较各个版本的差别，出了问题能回滚到以前的版本）的概念都没有，整个系统只有一个最新版本，一存盘以前的版本就被覆盖了，现在想来后怕不已。
1998年夏天，我到微软总部做实习生，第一次接触到了版本控制和代码审查这种高级文明，惊为天物。它们给我的印象如此深刻，以至于在三年后回母校的时候，我还把它们当作在微软学来的先进经验在曾经工作过的科大恒星公司（现在是上市的科大国创）大力宣讲，同学们忽闪着大眼睛，听得相当好奇。
代码审查是依赖于版本控制系统存在的。没有版本控制，所有的源文件都只存最新版本，没办法和旧版本对比，审查也就无从谈起。我最早接触到的版本控制系统是在微软用到的 Visual Source Safe 。这是微软开发工具 Visual Studio 早期的一个部件，只支持小规模的代码库。微软自己的代码量对它来说是太太太大了，所以大部分代码是用另一个性能更高的命令行式系统 Source Depot 来管理。不过微软不同产品有不同的代码库，我实习时在的 MSIT（微软内部技术）组的代码量小，用 Visual Source Safe 就可以了。
如何审查呢？在本世纪才开始工作的朋友可能不会相信：每次写完一个功能，我就把我的 mentor（负责指导我实习工作的同事）请过来，把代码打开，鼓动如簧之舌，介绍我改动的目的和实现方式。同事提点问题挑点刺，我再当场修正。总之把问题都摆到桌面上来，下面就好进行了。你来我往几次，最后大家都满意了，就按下一个按钮，把新版本正式加入到代码库中。
这种面对面交流的方式虽然简单易学，但是存在不少问题：
必须要两人同时在场才能进行。有时为了凑到一起，不得不打断正在做的事，增加了任务切换的成本。现代社会为什么出轨这么多？因为技术的发展实现了撩妹异步化，不用面对面，机会多了啊！要提高代码改动的吞吐量，必须实现审查异步化。
对于复杂的项目，经常需要深入的思考才能真正理解一个系统要解决的问题和它的设计实现，然后提出中肯的意见。突然面对面，啥事都要当场拍板，没有时间做深层次的思考，难以形成高质量的意见，往往沦为走形式。
好的代码，应该是无需解释就清晰明白的。原作者在旁边给你讲懂了，不代表这个代码让人一看就懂。而且被作者牵着走，容易犯同样的错误。这种审查方式，没有给审查人一个先独立思考自行理解的机会，会漏掉一些可读性的问题，甚至重大设计问题。
有的审查人碰到自己不太懂的地方，不好意思当面承认，也不可能当场学习调查，只好稀里糊涂盖个橡皮图章。就像奥斯卡影帝小李子在电影《有本事你来抓我啊》（Catch Me If You Can）中演的那个百变骗子，冒充外科大夫的时候，别人问他的意见他总是先反问别人怎么看，然后说“我也这么看！”（I concur!），别人听了很受用，就不再深究。这样屡试不爽。
审查的过程没有形成文字记录，后来的人只看见代码这么改了，却不一定搞得明白这么改的原因。甚至当事人过一段时间自己都不记得了，多年以后看得泪流不止。就像那个老笑话所言：张丞相好草书而不工，时人皆讥笑之，丞相自若也。一日得一句，索笔疾书，满纸龙蛇飞动。使其侄录之。当波险处，侄罔然而止。执所书问曰：“此何字也？”丞相熟视久之，亦不自识。诟其侄曰：“汝胡不早问，致余忘之。”
电影 Catch Me If You Can 截图
一般说来，如果项目简单程序员水平很高，这种方式还行得通；碰上复杂的项目或是程序员水平参差不齐的情况，结果堪忧。
当时微软有位著名的华裔程序员 Raymond Cheng，他写了一个工具来解决这个问题。这就是 bbpack（build buddy pack）。它可以把对多个源文件做的修改打成一个包，然后通过邮件发给审查人，让审查人在自己的机器上把包打开，不慌不忙地比较新旧版本的区别，甚至关上门悄悄问谷歌，把问题搞懂。等审查人把问题都想清楚了，再把意见写下来发给作者。这个工具解决了面对面的最大痛点，据我所知在微软用得很广泛。
我在谷歌上班之后很长一段时间，用的是类似的方式。虽然强于面对面，这还是有很多不足。最大的困扰是代码和意见是在两套完全隔离的系统中（前者在版本控制系统，后者是邮件），很容易脱节。一个复杂的改动，可能涉及到很多文件，每个文件也可能改好几个地方。所以，审查人可能会提很多意见。如何搞清楚哪条意见是针对哪段代码呢？我们采取的是原始的方式：直接写下文件名和行号。比如有一天我的审查人说：
老万，这个 photo/view/people.cc 文件的第135行到137行，应该做一个优化，把人脸按颜值先排个序再显示，确保你和马云的头像都出现在倒数第一页，这样可以提升用户体验。
（好吧，我们程序员排序，缺省都是从低到高的。不过马云是怎么回事？）
图片来自 www.gexing.me
那好，我去做了这个改动，然后就发现，这个文件行数变多了，因为以前三行的代码，现在改成了三十行。于是，那个文件后面所有的行号都不正确了，要相应地往后挪。如果审查人要求我在10个文件中改20个地方，光是数行号就够我忙活的了。所以，我被迫养成了一个习惯来适应这种原始的生产方式，就是倒着改：先解决文件尾部的问题，再顺着往前改。这样即便后面代码增加了或者删除了，不影响前面的行号。然而这种做法不一定总是可行，很多时候做一件事需要修改不同的文件，为了完成文件 A 尾部的一个修改，不得不同时修改文件 B 的开头和文件 C 的中间，所以没法保证所有文件都是从后往前改的。意见多的时候，还是经常数不清行号。此外，一次代码审查经常需要好几轮的讨价还价，一次改不好再改一次。所以不光是要比较两个版本，而是每轮都有一个版本。如何记得哪些意见已经被处理过了，哪些还是正在处理的？双方的负担都很重，效率低下。
怎么办？程序员是这个星球上最爱开发工具的生物，那我们就做个更好的工具呗。几年后，Python 语言（按codeeval.com 的统计，Python 已经连续5年是最流行的编程语言，超过了 C/C++, Java, C#, …...）的发明人 Guido 老大加入了谷歌。他老人家一看，这么烂的工具怎么能忍！作为投名状，他开发了一个基于网络的代码审查系统。因为 Guido 是河南，咳咳，荷兰人，他给系统取了个自己的老乡，荷兰画家 “猛钻”（Mondrian）的名字。这个系统大家用了都说好。后来 Guido 把它开源的时候，改了个名字。他还是心系祖国，这次用的是一位荷兰建筑师的名字，叫 Rietveld (关于这段历史，更多故事请看 )。相比之下，我做系统只会取像 Google Test 这样直白的名字，真是一个没有教化的野蛮人。
Mondrian长这样，真的不是张震投胎？我不信我不信！（图片来自wikipedia）
这个“猛钻”代码审查系统，比之通过邮件来交流的方式有天壤之别。它直接和版本管理系统集成，在浏览器上同步显示审查意见和代码，从审查开始到结束的 N 个版本，可以任选两个对比。每条意见，系统会开一条线索跟踪，可以不断追加后续意见。处理完的时候，可以点一个小框确认，下一轮审查这条线索就不再出现。对于我这样的审查话痨来说，这真是一款极大提高了生产力的神器。Guido 离开谷歌好几年了，但是 Mondrian 的升级版本还在被不断使用改进，我每天都在上面杀掉不少时间。
下一次，我想谈一谈多年审查代码积累的一些技巧和小经验。敬请关注。
往期文章推荐：
长按 - 识别二维码关注“老万故事会”公众号：