代码改变世界

大家是怎么做Code Review的?

2011-04-09 19:25  菜阿彬  阅读(7825)  评论(25编辑  收藏  举报

  先说说我们公司现在的做法,一个团队被人为地分为两个阵营:Senior Developers和Junior Developers,比例差不多是1:1,Senior Developers就担负着对Junior Developers的代码进行Review的职责,每天Review一次,对有问题的代码写上comments,然后也check in到代码库中。这种comments有特殊格式(比如//\\CodeReview:blah blah),要求Junior Developers每天下班前一小时去代码库中找这种格式的comments,然后修复自己的有问题的代码,修复时删除Reviewer留下的Comments。把Review的Comments也check in到代码库,本意是说让任何东西都有记录。

  我对Code Review的理解是,最好的形式是“结对编程”,特地查了下Wiki对Code Review的定义,明确讲了“Pair Programming”是Code Review的一种形式。我个人非常喜欢pair,甚至到了在写一些重要代码时,没人跟我pair我都不想写代码的程度。然而现实中,大部分人对pair是排斥的,尤其是那些对软件开发似懂非懂的项目经理们——他们直觉上认为那会让生产力减半。

  那就回到现实,在一个不允许进行“结对编程”的团队里,怎么做Code Review 比较好呢?说说我的想法:

  1,Code Review在某种意义上是一种“反馈系统”。软件开发中存在太多的“反馈系统”。尤其是在“敏捷”中,追求更多的“反馈”,也追求更快的“反馈”,比如Iteration,Daily meeting,TDD,face-to-face communication……不一而足。“反馈周期”总是越短越好,“结对编程”的反馈周期为0,它是“即时”的反馈系统,这也是我认为它是最好的Code Review的原因之一。那么除开“结对编程”,怎么让Code Review的“反馈周期”变短呢?像我们公司一天一次Review的做法,也许已经很短了,但显然还有更快的,比如每次Check in时Review。

  2,check in前review本身也不一定就更快,如果有人2天才check in一次代码,那还不如daily的review呢。所以就需要结合另外一个理念:check in as often as possible。以我自己的经验,在有活干的时候,check in频率在30分钟到2小时是正常的。有时会超过2小时,但那要么是有特殊原因,要么我就感觉自己写代码的方式出了问题。

  3,顺便说句题外话:看到不少程序员的一个“通病”是,完全没法在短时间内check in,他们一旦开工(尤其是做一个比较大的模块时),一发不可收拾,这边写一个类,还没写完,突然又想到要写另一个类,这样写了很多代码,你过去让他check in,他会告诉你再等等,因为他的代码现在还没法通过编译(这跟把没通过编译的代码check in的人比,还是很有人性的)。事实上,一个高手的必备技能之一就是让自己的代码处于不安全状态的时间越短越好。这种不安全包括不能通过编译,不能通过现有的单元测试,不能通过整个应用程序的smoke test……。对有这种“通病”的程序员来说,TDD是剂良药,我个人也认为他是一个分水岭:这世界上有两种程序员,一种会TDD,另一种不会TDD。个人意见,就不展开叙述了。

  4,我们公司的做法还有哪些问题?作为Reviewer,我要去找哪些代码在今天被修改过,这很费时间,如果有个工具帮忙会好点,比如有些系统可以把每次check in产生的diff发送mail给reviewer。但是,很多时候,事情的本质不会通过一个工具得到改变。本质是什么?是这种Code Review的做法是基于“push”的一个做法——Reviewer被push去review代码,然后把Review的Comments提交到代码库,Junior Devs被push每天去搜索那些comments,然后修复它们。

  5,有没有一种基于“pull”的做法?我以前一家公司,有设置一个简单的check in policy:每次check in时,必须在note里填上reviewer的名字。这样,每个人check in前就会主动找人给他Review,我感觉这有点“pull”的意思。

  6,另外一个问题是:比如某人写了一天的代码,Reviewer在第二天Review后发现他的代码设计有比较大的问题(但是能work),那么在第三天,这个人会去重构他的代码以达到更好的设计吗?基本上是不会的。那这样的review有何意义呢?

  7,这里牵涉到Code Review到底要Review什么的问题,我觉得至少可以分三种:一是小的issue(比如命名规范,代码标准);二是大的issue(比如内存泄露);最后是那种非“issue”,而是设计是否优雅简单,代码是否干净可读的问题,这种问题不会导致程序出错,在短期内甚至没有任何问题,只会在一段时间之后引起维护成本,可扩展性之类的问题。我觉得越是一个成熟的团队,Review时花在最后一种情况的时间会越多。可是这种东西,有时没有一个标准,更多的是一种程序员之间的交流和互相学习。而把Reviewer的comments冷冰冰地记录在“不良代码”的上方,然后被Review的人每天默默地修复那些“不良代码”的行为,是不是完全忽视了这一点呢?

  8,总之,code review要注意达到一些目标:快速反馈,简单有效,知识传播……

  一些零散想法,不成体系。不知道大家在公司有没有做Code Review?是怎么做的呢?