专栏名称: 待字闺中
深度分析大数据、深度学习、人工智能等技术,切中实际应用场景,为大家授业解惑。间或,也会介绍国内外相关领域有趣的面试题。
目录
相关文章推荐
程序员小灰  ·  我发现凡是给offer的公司,面试时基本不问 ... ·  6 天前  
中国能源报  ·  能源早新闻丨赵荣科被查 ·  3 天前  
中国能源报  ·  能源早新闻丨赵荣科被查 ·  3 天前  
深度学习这件小事  ·  有想入坑RL-LLM的同学吗?这个开源项目一 ... ·  3 天前  
51好读  ›  专栏  ›  待字闺中

也谈代码审查之道(路)

待字闺中  · 公众号  · 程序员 科技自媒体  · 2016-09-10 00:46

正文

作者简介:叶顺平,现在是国内某个著名的创业公司的技术总监,毕业于北京大学。他是待字闺中特邀作者。我们也希望有更多的作者投稿。


昨天读到陈老师的文章《我的“Code Review”成长之路》,觉得陈老师的总结可谓高屋建瓴,句句都是经验之谈。陈老师之前在谷歌工作了很多年,回国后联合创始了创业公司云壤,有幸我也在云壤,因此我也有不少代码,需要经过陈老师的review,从他review的comments中学习到了很多。正如他文章标题是成长之路,他在审查技能成熟后给我做了很多次审查,正好是我成长之路的开始,可谓首尾相接。其实代码审查的成长之路正是如此,大家经过长者引路,慢慢走向成熟,然后review其他人,代码审查之道也得以传递和发扬。


这里不揣冒昧,也来凑凑热闹,聊聊我的代码审查之路,兼谈代码审查之道,也算是对陈老师文章的补充和有趣的code review话题的延续。

原始冲动

在加入云壤实习之前,我在一家只有二三十个人的创业公司实习,团队最开始的时候甚至只有五六个人。去这家公司实习之前我写的代码非常有限,公司又缺乏高手,因此很多问题都是自己摸着石头过河。当时做的是一个windows项目,类似目前于QQ的IM软件,使用Win32 API直接写,而不是使用MFC等高级别的界面框架。QQ大家天天在用,看似简单,不过界面,换肤系统,文本聊天,文件传输,视频聊天,音频聊天,登录系统等,整个系统的复杂度也非常高。当时使用了很多的第三方软件,而我在之前并没有开发过组件如此多的项目,整体项目复杂度超出我的掌控能力。由于组里缺少有经验的工程师指导,最开始我们疲于开发新功能,系统总是有了这个功能,那个功能又出了各种Bug,可用性和稳定性很低。当时组里没有代码审查的,在项目进展了一段时间后,我拉了几个同学过来,项目研发团队达到六七个人左右。慢慢地我意识到了代码审查的必要性,开始尝试确定小组的代码风格指南,不过风格指南不成体系,也没有参考业界常用的几个标准。代码审查也比较原始,一般是合并进去后,在Windows IDE里使用SVN的插件,看看之前的更改历史,然后把相关的开发叫过来,或者我坐到他身边去,做面对面沟通和审查。现在回想起来,一个项目流程的完善程度,code review标准的严格与否,其实和公司规模没有什么关系,和人聪不聪明也关系不大,而是有没有相关的经验。


这算是我职业生涯初期的代码审查,原始而落后。有很多痛,痛过后有需求,然而只是面对面交流代码,不依赖工具,不依托平台。风格等完全领导者经验总结,不成体系。我相信国内大部分公司的现状应该也是这样的。

云壤的代码审查氛围


幸运的是,在一年后我加入云壤实习。在这里,code review是全公司的一种文化,后端代码要review,前端网站,安卓和iOS等项目的代码也都会review 。code review不仅是一种工程师文化,更是技术大牛们日常喜欢做的事情,缺少了code review,只是写代码,就类似于好作家只写文章,却没有可供阅读的其它文章一样寂寞。

在云壤,从CTO到各个技术VP,技术总监,每一个都写代码,也每一个都会做code review, 他们每天花费很大的精力在代码审查上。甚至有个小故事,有一天CEO拉住某个研发,说我昨天翻看你代码,你代码里字符串匹配的效率太低了,然后邮件发送了一篇论文,介绍了高效的字符串匹配算法(搜索里经常需要使用N个关键词去匹配某个网页的正文)。当然,代码审查这个事情,也有做的不够好的地方,比如有的小组负责人不是谷歌出来的,可能这个小组的人的大部分代码就审查得不够严苛。后来我觉得类似的问题和风险,确实应该通过入职时全公司范围的一次code readability活动来解决。让经验丰富,非常熟悉coding style的同事把关,才能够解决国内大部分公司的问题。云壤当时的后端团队不算大,前谷歌的成员就有近十个,比例算比较高了,但是依然会存在一些同事半年一年过去了,他的代码还是各种风格问题和设计问题。

下面说说我自己被review的经历。最开始的时候,大部分人新加入一家公司,总是希望尽快完成第一个项目,急于提交自己的代码。假如没有像谷歌那样完善的基础架构和工具文化,大家写完代码就提交了,可以绕过review流程。比如SVN不像git那样大家工作在不同的分支,就非常容易不做pre-commit的code review 。我的第一个项目,也是急于求成,很快就被我偷偷提交到代码仓库中。后来一个谷歌同事发现了,要求我提交code review, 我说已经提交了怎么办?后来同事就坐在我旁边,打开我相关代码,一处处给我讲应该怎么改,哪些代码有风格问题,哪些地方可以修改下架构等。在这之后,我才意识到代码不能胡乱提交,需要至少得到一位reviewer的LGTM才能提交。(后来我在做reviewer的时候,也碰到类似自己当年一样顽皮的少年,搜索方案后才知道,其实SVN中,就是代码提交了,也可以做post-commit review的,这是后话)。

Code review虽然一路被虐,不过能得到严格的代码审查,自己也是非常开心的。聪明人都希望挑战自己,从错误和失败中学习。慢慢地,我的leader告诉我,写代码和写文章一样,在提交代码前,最好先自己review两遍,这类似作家在发稿前三易其稿是一个道理。SVN或者git也都有相关的插件,方便在本地做两个窗口左右对比的diff,自我审查,向自己开炮。

代码审查的时候,有很多事情可能让你不爽,比如,这个变量名名字不够好,但是你觉得还不错。这里注释第一个单词应该大写开头,这里到底应该是URL还是Url, 你作为代码编写者,总觉得无所谓,reviewer是在吹毛求疵,没事找事。然后当你在下次编写代码的时候,也会自然遵守,省得以后给自己找麻烦。后来慢慢地,你可能发现,从代码管理者,从整个项目,整个Repo,整个公司的角度看代码,URL还是Url会出现很多次,假如你这里是A,别的地方是B,就会成为一个问题。很多鸡毛蒜皮的事情,在一个人的习惯里,可能是半斤八两各有利弊,如果只见树木不见树林,确实容易发生争吵。然而,假如你的编程生涯中,能有一位同事因为代码风格而和你争吵,那是你的幸事。

当然,如果你的代码修改了十几行,却收到了几十条修改意见,你除了沮丧外,也会感到高兴,今天终于学到了几招,比如你会发现:原来代码可以篇幅节约一半,原来变量的命名这么讲究,原来一个函数名可以节省好多注释,原来一个好的风格,不写注释大家都容易看懂,原来BUILD文件里的一个关键字藏有这么多的隐秘,原来某同事早已经写过这么通用而优雅的基础库。如此等等。

有时候反过来,你的Leader,或者更高级别的人的代码,也会让你来review, 因为你是某个模块的Owner,你最熟悉。这时候如果你反过来作为reviewer,能够发现他们代码的一两个问题,也会享受这种成就感,“哦, 我终于也能发现大牛的代码问题了!”。你会发现,大神离你其实并不远,大神也不是一遍通过编译,一次写出华章,这和曹雪芹才高八斗,红楼梦却也是数易其稿一个道理。


搭建review环境


后来我去了一家公司,这家公司没有代码审查的文化,技术领导者在这方面也没有太强的意识。然而,在云壤的一年,已经让我意识到,不仅是我自己的代码需要被同事review, 我要掌控组里的代码质量,提高大家的工程能力,代码审查流程也是必不可少的。而代码审查这个事情要得到落实,就需要做几个事情:

1,确定code style guide。这个我们选择的是Google c++ Style Guide。
https://google.github.io/styleguide/cppguide.html

顺便说一下,现在我所在的创业公司,我们Follow谷歌的大部分风格指南,包括Java和Python等。


2,  清理现有代码里的风格问题,尤其是基础库。

3,把控第三方库的选型,尽量选择风格比较一致的项目。

4,搭建代码审查平台。这个我们当时选择的是SVN + review board。review board项目地址如下:
https://www.reviewboard.org/

如果是git的话,gitlab也自带一个代码审查的功能,不过比较简单,有很多小问题。当然gitlab也有方便的地方,比如可以在网站上完成code merge.

5,完善工具,比如自动化的代码走查工具或者脚本。C++有google的cpplint.py,地址如下:
https://github.com/google/styleguide/tree/gh-pages/cpplint

当然,除了检查脚本,vim, eclipse等IDE也可以配置相关的formatter, 尽量减少调整格式需要的时间。

6,给组里的每个人做严格的code review, 慢慢培养成熟的reviewer。

工欲善其事必先利其器,自动化脚本和IDE formatter的作用还是很大的。如果有可能,可以对gitlab, bazel build tool做一些二次开发,在开发流程中,更好地落实风格检查,从要求,到文化(习惯),到工具性约束。这也就是陈老师文章中说到的,要法治不要人治。人治固然在有明君的情况下能够政治清明,但是作为聪明人,谁会希望自己累死累活,每次review都纠结一些鸡毛蒜皮的style问题呢?更何况是能自动检查出来的问题。

另外, 国内的开发环境比较复杂,就是BAT落实代码审查的团队也少得可怜。因此团队里哪怕很多BAT大厂出来的开发,要推行较为完善的代码审查流程,也会遇到很多阻碍。假如遇到不配合的,那你要做的就是,告诉他事情的重要性,如果一定不遵守,那就不要他的代码。当然,只要review是好的,review的建议是有价值的,一般都会得到重视,相应的开发也会及时Fix,然后重新提交review request。


在review中学习

代码审查这个事情,如果说被别人review是幸事一件相对比较好理解的话,那么做reviewer也是幸事,可能相对不好理解。因为reviewer和被review者的关系,往往容易被误解为高手和非高手,高手和菜鸟的关系。这和作为面试官,其实也能学到东西是一个道理的,不过一些人可能觉得面试比较浪费时间,属于高和低的关系,面试者只输出不输入。而这,是最需要被首先纠正的错误观念。

被别人review, 你自然能够从中学习到你的代码风格问题,你的取名字的艺术怎么样(变量名,函数名,类名,命名空间名等),你的组织语言的能力,代码结构,是否言简意赅(算法简单,代码冗余度低)。能够学习到是否前人已经创造过这个轮子,并且做得更漂亮更高效,能够学习到更简单的笔法,学习到效率更高的算法,能够学习到各种异常的处理,甚至学习到怎么使用benchmark程序来给自己的代码算算性能,学习到怎么编写合理的测试用例,用于发现隐藏的Bug(比如各种边界条件),能够学习到解决问题的更高效的算法,了解到更奏效的文献。被review的越多,你自己自我审视的眼光也会越严苛。

然而,自己不容易犯错,不代表你能教会别人少犯错。正如一个好的学生不一定会成为一个好老师。这需要训练,需要有意识地让code review成为较为系统的方法论。在做review和被review的过程中,不断总结,阅读代码重构和《代码大全》之类的文章,理论结合实践,深化理解。陈老师在文章中提到可以列出来一个Check List,这个当然是不错的法门,不过做的code review多了,这个check list就会隐藏起来,化而不见,就仿佛高手手中无剑一般。

学习review别人的代码,我认为首先应该从review自己的代码开始。只有自己的代码问题,能够大部分消灭于被review之前,才能够养成毒辣的评审眼光。看别人的代码,可以从代码风格,可以从代码结构,可以从编译命令,可以从代码复用等不同的角度看,也可以从算法,从与其它模块的耦合性等看,总之,看明白代码的意图,想明白自己实现的话会怎么做。知之为知之,不知为不知,有问题的地方直接指出来;有疑问的地方写出来;有不懂的地方说出来。你看不懂,说不定未来他自己也看不懂,接手维护的人也看不懂。如果代码真的很复杂,那也写出来,让开发者写上注释,放在相关资料链接等。他日如果需要重构,才知道从何下手。

从严格的review角度而言,review代码和写代码的活动,其实非常相似。review代码虽然不用写,但是也需要想怎么写,怎么重构,阅读的过程需要用上自己在写的章法和布局。

陈老师提到的很多原则很好,我自己作为reviewer也在阅读的过程中发现了一些不足。比如一些代码其实不太懂,但是我作为reviewer,不能够虚心求教,或者花一些时间查阅相关的资料,只是做一些浅层次的review,走马观花,只是关注代码美观和简洁性等表层。但是内部逻辑,异常处理,算法层面却未能提出自己的建议。作为一名reviewer,给别人成长的同时,其实也是给自己成长的机会。Review一些陌生的模块和领域的代码,和挑战自己去开发一些不同领域的软件一样,同样需要勇气。可见,reviewer本身也是需要不断成长的,才能更好地胜任reviewer的角色。

幸运的是,我现在所在的创业公司,也是一家谷歌范的技术公司。AI相关的几个领域,包括语音识别,NLP,搜索,语音合成,图像处理等,我们的主流开发语言和Google一样主要是C++,我们也follow 严格的Google Style Guide。作为reviewer,我们经常在跟进style guide的更新,也在跟进C++等语言的新标准。近来技术圈讨论CTO要不要review代码讨论得很火爆,虽然我觉得CTO做不做代码审查都可以,各有各的好,毕竟精力有限,用在这里多了用在那里就少了。不过我个人还是对坚持review代码的CTO更有好感。我们的CTO也和陈老师一样,对谷歌式的code review有着近乎宗教版的信仰,并且每天花不低比例的时间在code review这个事情上,新人入职的第一个commit,一般都会热心地做一次较为详细的review。CEO在公司早期做代码审查也非常严格,听说最早的几个员工,经常被他review到面红耳赤,近乎崩溃,然而也正因为严格,最早的几个员工成长非常快。

我希望在代码审查这条路上,能越走越远,越走越悟道。十年之后,我希望自己还能经常写代码,有时间和好心情做code review 。据说谷歌出来的很多老员工,也经常会做代码审查,听朋友说起来,对他们的review之勤,也是赞誉有加。可见谷歌出来的那些人,确实热衷代码审查,也乐于传播代码审查文化。虽然我自己没有在谷歌工作过,但是也受益于身边很多谷歌人的这种代码审查文化。未来希望这样的文化,不仅仅是在类似云壤这样的谷歌系的公司里得到传播和发扬,也能够在国内更多的现有大公司里,创业公司里得到发扬。这样保不定程序员朋友们哪天换个环境,我们会因下一站有类似的工程文化追求而添一份亲切和温暖呢?


大家可能还感兴趣:



谢谢。