老万故事会

【老万】我在 NB 公司审查代码

码工的日子真是没法过了。

早几年互联网大热的时候,一茬一茬的公司竞相招兵买码,程序员都是香饽饽。一看码工工作好找挣钱还多,其它赛道的同学们都齐刷刷地刷题转码,又齐刷刷成功上岸。几年下来,这个行业变得异常热闹。

可惜,人无千日好花不常开,有一天大家学习了一个名词叫“经济下行”。忽如一夜冬风来,千村薜荔人遗矢。大厂们从抢人秒变裁员,翻脸比乾隆爷翻牌子还麻溜。这种情况下,码工人人自危,生怕半夜收到人事部的邮件。

我在 NB 公司做过一阵子技术领导(此事详见鄙人《我从 NB 公司技术带头人的位置上离职》一文)。一天,组里来了个中国小伙叫刘立强,老板把他分到我负责的项目。小刘博士刚毕业,NB 是他第一份工作。虽说他年纪不大资历尚浅,但长相敦厚,一看就是老成持重老实巴交的老黄牛体质,所以就被叫成了“老刘”。

老刘编程基本功扎实,只是在公司做事经验还不丰富,需要我时不时点他一下。不过,他做事用心,一般来说只要我指个方向,他就能很快交出答案,我倒也省心。

众所周知,我是一个面慈心善以德服人的人。老刘既然是个好苗子,我就豁出去好好培养培养,我也好有自己的得力干将。

一天早上,我到公司后把电脑包放下,先去厨房转了一圈,拿了一瓶椰子奶两包豆腐干三块巧克力。回到座位,先不急看邮件,我撕开豆腐干的包装,一边嚼,一边查 Nvidia 股票的行情。

乖乖,比昨天又涨了 6%,早知道昨天就买了。不,要是十年前我把买房子的钱拿来买 Nvidia,现在十栋房子都赚到了。那会儿怎么就没有哪位高人点拨我一下呢?现在入场会不会被高位套牢?

正在纠结,一个戴眼镜的家伙跑到我桌子前闯进了我的余光中。我一看,这国宝状笑容可掬的除了老刘还能是谁。看样子,是有啥急事。

“什么事?”我问道。

老刘:“万总,您昨天布置给我的任务,我刚写了一个 PR。您有空帮我审查审查呗。”

Image

PR 就是程序员写的改动(pull request)。大厂就是事多,写完代码还得审查才能入库。做为技术领导,我少不得要审查他人的代码。

我说:“知道了,我现在有点忙,事关,嗯,事关人类的命运。下午我会看。”

Nvidia 关乎 AI 的未来,AI 关乎人类命运,所以我关注 Nvidia 股价就是关注人类,说得没错。

老刘还是年轻,让我一眼看出了他脸上流露出来的失望。突然想起离上次跟老刘推心置腹已经是三天前的事了,我心又软了。

“要不这样,反正人类的命运一时半会也操心不完,我现在就看你的 PR 吧。”

老刘喜上眉梢:“感谢万总!”

我示意老刘把办公室门关上。接下来的话都是我多年积累的经验之谈,不能让别人随便听去了。

~~ 我的代码审查之道 ~~

我在电脑上打开了老刘的 PR。一眼看到的,是他写的一页变更描述(PR description)。我大致扫了一眼,不由一声叹息。

老刘说话都抖音了:“万总,是不是我写的不合要求啊~啊~啊?”

“老刘,你别紧张。我先问你,知不知道这年头对我们码工最重要的是什么?”我喝了一口椰奶。

“是什么?”老刘把问题又扔了回来,一脸天真。

我高深莫测地一笑:“最重要的是:不要丢工作。现在就是他们说的历史垃圾时间,公司钱包捂得紧,咱们的焦点就不要放在升职上了。特殊时期,保命要紧,咱得确保自己不出现在下一轮被雷的名单上,尤其是你这样还没拿到绿卡的情况。你说对不?”

老刘点头如捣蒜:“对对对!怎样才能确保呢?”

我神秘地一笑:“那就得让自己的工作不可替代了。依我看,你这 PR 描述犯了三大错误,你这是在跟自己作对啊!”

“请万总明示!”老刘挺直腰板说道。

“第一,你标题起得言简意赅,把你要解决的问题清楚地点出来了,让人一看就对这个改动的目的了解得八九不离十。”我循循善诱。

“这有什么不好吗?”老刘露出迷糊的神情。

唉,真是块榆木疙瘩,看来我得把话说透了。

“唉呀老刘,你想想,你写得这么清楚,不是便宜了别人吗?你看,要是小麦要接手你的项目,是不是一看你的那些 PR 三两下就能大致搞清了?反过来你要是接手小麦的工作,就冲他 PR 描述写得那狗屎样还不得要你十天半个月啊?要是老板有一个雷人指标,你们两个必须开一个,还不能影响项目进度,你说他会开哪个?”

老刘终于开始明白了:“哦,原来我废劲巴拉地是在给自己挖坑啊!”

我:“所以嘛,干活得留一手,不能傻乎乎地把啥都交代清楚了。你呀,得把自己练成刘一手。”

老刘:“那您看该怎么改呢?”

我略一沉吟,有了,上手把“将NBHH系统查找智商最低用户前k名的时间复杂度从平方级降为对数级”改成“一个优化”。

别的不敢说,这样起码把小麦找到这个改动的时间复杂度提升了一个数量级。我好像看见小麦一脸生无可恋的样子。

再一看老刘,脸上满满写着“这也行?”

“年轻人,这有啥不行,为了保住饭碗,PR 标题越语焉不详越好,下次可以试试‘fix一个bug’、‘实现一个功能’这样的。”

看到老刘点头,我欣慰地想:思路打开了,问题就好制造了。

“万总,您刚才说有三个问题,还有两个呢?”

“第二个么,就是你描述的内容。你看,你上来就开门见山解释了为什么需要做这个改动,然后还分析了新算法的原理和你选择它的原因,甚至你还写了对最终用户体验的影响。最后,居然还详细解释了你是如何测试这个改动的。”

我一边说一边摇头,说得老刘低下了头。

我想了想,又补充道:“这些都是最最重要的信息,你怎么就随随便便给人了?也太不把自己当回事了吧。我建议,你把现在的描述删了,要么换成让人摸不着头脑的实现细节,要么换成正确而无用的废话。对了,你可以让 ChatGPT 帮你写,它干这个可在行了。”

老刘掏出一个小本本,工工整整地写下“成功秘诀:一本正经的胡说八道”。

嗯,这就是我喜欢这小伙子的地方。孺子可教啊。

我一看老刘把我讲的重点都消化了,便继续分析:

“第三个问题,就是你书写的形式。叫我咋说呢?居然挑不出语法和拼写错误,大小写都写对了,句子结尾的句号也没有忘。从结构上说,条理还很清晰 - 这就是你不对了。你知道后果吗?”

老刘:“太清楚了别人容易看懂?”

我点了点头:“嗯哪,你想想,日后会有多少人在维护系统的时候读到你这段说明。不给他们制造一点阅读障碍,你不就是在给他们取代你助力吗?什么叫亲者痛仇者快,这就叫亲者痛仇者快。”

这是老刘的脸已经比新瑞华的油焖大虾还要红了。我看见他又在本子上写下:“男儿当制障!!!”三个惊叹号分外醒目。

既然他这么好学,我就扶上马再送一程。

“老刘,把改动解释清楚还有一个恶果,就是让一些自以为负责的代码审查人更容易提出改进意见,甚至质疑究竟有没有必要做这件事。他们可能会无事生非,提出一个他们认为更高瞻远瞩的方案让你实现。你看,你这不是自找虱子放到自己头上吗?”

“可是,”老刘迟疑地问道:“问题不是越早发现越好吗?”

“唉呀妈呀,你还真的跟资本家共情了?”我气不打一处来。“老刘啊老刘,跟你说过多少次了,咱的首要任务是保证自己职位的安全,至于系统的质量,那是高层领导需要操心的事,跟你我有什么关系。他每年领的成堆的股票,是分给你一分一毫了吗?”

老刘一边点头,一边飞速写下:“介绍以乱麻状为上佳,让审查人越头大越好,切忌让他们想清楚改进方案。”

看完 PR 描述,我飞速扫描了一下 PR 的内容。我们的 NBHH 系统是用 C++ 做的,老刘一共改了十几个源文件:三个头文件,还有十来个 .cpp 源文件,其中一小半是测试。

我给三个头文件打上星标,从它们看起。一边看,一边跟老刘解释:“看别人的改动,千万不要按字母顺序挨个文件看。擒贼先擒王,咱们要抓住重点。什么是重点?关于编程接口的改动就是重点。对 C++ 来说,就是要先看头文件,因为它定义了一个模块有哪些类(class),这些类又支持哪些操作。”

又说:“要确保自己的位置不被别人取代,咱们首先要把接口搭好。具体地说,要有足够的烧脑度。要是接口清楚直接,朗朗上口,具体实现就不用看了,因为你已经输在了起跑线上。只有把接口搭得横七竖八犬牙交错,你在做实现的时候才有发挥的空间,才写得出惊天地泣鬼神的代码。”

说到这里,我指着老刘新添加的几个函数声明,说:“你看,你这几个函数,命名太准确太规范了,缺乏一些新鲜感和刺激性。更要命的是,参数的顺序竟然符合逻辑和英文表达习惯,让读者看到调用这些函数的代码时不需要查阅手册就能把它的意思猜个八九不离十,完全起不到拖延友军的作用,把大好的机会都浪费了,啧啧。”

看到老刘奋笔疾书,我来了兴致,索性送佛送到西,又说:“你知道铁饭碗编程的三大要素吗?”

不等老刘回答,我继续道:“错误的抽象、不知所云的抽象、似是而非的抽象。”

老刘露出了不知我所云的表情。

我站起来,活动活动肩膀,又朝嘴里倒了两片豆腐干,嚼吧嚼吧。我这是给他一点时间消化消化,让他自己悟。

“明白了!”老刘叫出声来:“抽象的对立面就是具体。所谓编程接口,就是一种抽象:把各种操作的具体复杂实现包装起来,再给它们取几个简单的名字,以后做这些操作时,就不用再啰嗦那些实现细节了。”

“所以呢?”我继续引导。

“所以如果我们把函数名或者参数取得名不符实,或者让人摸不着头脑,或者有歧义,又或者彼此不一致,就会让使用这些函数的代码像八宝粥一样一团糊涂,进而让维护这些代码的同事也如同盲人骑瞎马,任他们参到天上人间去也参不透!”

老刘越说越兴奋,我也频频点头。我没有看走眼,真是棵好苗子啊!

等老刘说完,我补充道:

“每个函数都跟它的使用者有一个约定:它的输入和前置条件是什么?输出和后置条件是什么?副作用又是什么?要是这个合约定义清晰,符合直觉,读者就能容易正确猜出它的用法,此乃程序员保命大忌。”

又说:“你看,你写的这个用户查询函数,除结果外还返回一个错误代码,你还加了注释,清楚地解释了函数的合约,让读者一看就知道调用者需要处理出错的情形,这就不太好。不如在返回结果中去掉错误代码,让它出错时直接返回一个特殊用户,比如咱们的 CEO 纽斯克,就有出人意料的效果。”

又说:“还有,再加入一些匪夷所思的副作用就更好了,比如,要是查询结果包含多个用户就随机冻结其中一个用户的账号,岂不是很爽?”

老刘:“万总,我明白了,我会举一反三,一定把接口改得让读者三观震裂,甚至开始怀疑人生!”

等把老刘的接口梳理乱了,我开始点评他实现功能的逻辑。

可喜的是,老刘已经完全融会贯通了我的想法,进入了自动模式。只听他喃喃自语:

“以前,我以为好的代码要正确、易读、高效。今天听了万总的教诲,我才意识到自己这是黑白颠倒,是非不分。从前我错得可真是太离谱了。从今往后,我要提升自己在代码中埋坑的技能,以降低可读性为己任,在大老板能接受的范围内绝不优化性能。这样的代码,只有我能维护,老板绝不敢造次。”

“说得不错,”我嘉许道:“不过,在一些无关紧要的地方,为了提升系统复杂度和维护成本,做一些不必要的优化也是有必要的。”

“高见,高见!”老刘在本子上写下“优化必须让系统更复杂。”

不知不觉我们已经看到了老刘的测试代码。这傻孩子,老老实实地给所有改动的功能都加了测试,还把测试代码写得跟他以前的业务逻辑一样清楚。

“老刘,”我说:“测试代码跟业务代码有一点不同,就是没有谁会给测试代码本身写测试。所以嘛,这是一个绝佳的埋坑机会。你只要在测试代码中加入一些不必要的抽象和弯弯绕,很容易就能植入几个逻辑错误,这些错误极少有人能发现 - 谁会仔细读这些在生产场景中用不到的代码呢?有了测试中的错误,在实现中有 bug 也就不容易发现了。”

老刘诚恳地说:“感谢万总为我开悟!”

~~ 尾声 ~~

在跟老刘这次谈话后不到两个星期,我跟老板马克⋅安德生发生了一场冲突,结果大家都知道了。我离开了 NB 公司。

听我的前同事们说,我走后公司开展了整风运动,我呕心沥血教给老刘那些经验之谈,终究还是未能传世。

~~ 后记 ~~

如果你在阅读这个故事的时候感到极度不适,恭喜你,那就对了。万总在这里当了一次反面教材。在职场上耍这种小聪明,终究会害人害己。要保住饭碗,还是要靠努力提升自己的技能,把事做好。人在做,天在看,一点一滴的积累,最后会成就一个卓越的你。

万总审查代码的顺序是对的:从 PR 描述开始,再看接口,最后看实现。但万总给的建议几乎都是错的。如果这一点不明显的话,我们来总结一下正确的方法:

Image

从变更描述开始

首先仔细阅读 PR 描述,理解作者的目标(我们为啥要费劲做这个改动?)和重要的决策(我们为啥要用这种方式做?)还有预期的后果(对用户有何影响?)。如果答案不明确,要求作者重写小作文,直到讲清楚为止。

很多人对写 PR 描述这件事是抗拒的。但是,花在这上面的时间很值,因为:

1. 变更描述是最重要的元数据,将来维护代码的工程师在调试代码和学习代码历史时会反复阅读它。

2. 只有知道作者到底想解决啥问题,审查者才可能提出更好的解决方案,甚至发现这个改动是多此一举,避免无谓的浪费。

然后关注接口

等理解了变更试图实现的目标,就可以进入下一流程,开始阅读代码。

然而,不要老老实实按字母顺序挨个读文件。先看编程接口的改动(对于 C++ 来说,就是先看头文件)。如果接口搭得不好,审查实现部分只会是一堆无用功。过早关注细节只会拖慢每个人的进度。

接口优先还有一个好处:可以确保接口得到公平的评判。

比如,如果先读实现部分,那么在审查接口时就已经知道了每个函数的功能,这会影响审查者挑错的能力。受潜意识里这些知识的影响,他们可能会觉得一个混乱的 API(编程接口)很清楚,可能发现不了不准确的命名、不足的文档和不自然的合约。

编程中最重要的三件事是什么?抽象,抽象,还是抽象。

如果 API 提供了良好的抽象,使用它的代码就会易于理解和推理。逻辑会很自然,如果有错误会显而易见,读者很少会被误导。

好的函数应该有一个明确的合约(输入和前置条件是什么?输出和后置条件是什么?副作用是什么?),这个合约应当符合直觉,一个从未看过其文档的人应该能正确猜出函数的用法。

在代码审查的前几轮,重点关注 API 部分。除非作者已经把 API 做好了,不用浪费时间审查实际实现。反正接口改了实现也要推倒重来。这样,作者能快速得到审查者的反馈,大家都可以避免纠结于实现部分而做的无用功。

在审查 API 时,确保每个函数都有一个明确的合约,并且各个功能之间配合得当。做为审查者,只有在感觉自己可以自信地使用这个 API 编程时才应该进入下一步:审查实现部分。

最后审查实现部分(包括测试)

到了这一步,大多数设计选择应该已经定型,所以我们可以专注于代码的正确性、可读性和效率。当然,我们也可能在这一阶段发现一些之前漏掉的设计问题,这时便需要返工修正设计。

审查者应该确保所有重要的变更都被测试覆盖,并认真审查测试代码,就像对待业务逻辑一样。

注意测试代码在风格上会和业务逻辑有一些不同:通常没有谁会给测试代码自身写测试,所以测试代码必须格外简单直白,一眼看上去就是对的。因此,在测试代码中要避免过度抽象,以直接的表达方式为佳,哪怕有时会因此付出一些代码重复的代价。

除此之外,测试代码也需要经过深思熟虑,同样要遵循正确、可读、易于维护和高效的原则。

~~~~~~~~~~

猜你会喜欢:

~~~~~~~~~~

关注老万故事会公众号:

本公众号不开赞赏不放广告。如果喜欢这篇文章,欢迎点赞、在看、转发。谢谢大家🙏