代码审查

【编者按】代码审查,本身应该是一个相互合作,相互学习,整合团体动力,最终却以消极和敌意为代价向前发展。这种现象是如何造成的,我们又该如何克服呢?原文作者Erik Dietrich给出了一些见解。译文如下:

 

前不久,我收到一封有关讨论代码审查的邮件,对方对其抱着无所谓的态度,我想这可能是大部分人持有的态度,这也一直是大多数的代码审查的面临的尴尬状况,但不是全部。与其冒着把孩子与洗澡水一起倒掉的风险,那不如干脆不要把洗澡水弄脏。我的意思是,完全杜绝糟糕的代码审查。

鸡蛋里挑骨头和无意义的讨论

我想我们大多数人都同意,没有什么比花一两个小时的争论是否要使用隐式类型,或是抛出ArgumentNullException还是NullReferenceException异常更无聊了。为什么不让你的代码审查变成那样呢?你可以不用担忧大局,仅仅进行哲学讨论,如代码的可读性和易维护性。那些曾将代码审查作为学习和合作的机会的人,很快就会意识到,他们只会关注细节上主观意见分歧激烈的地方。

居高临下和嘲讽

在正常的代码审查中,一般会指出一个可能的逻辑错误,但千万不用止步于此。如果错误可以驱动开发,那么添加一句“是个人都不会忘记这点”的评论会让他们更加印象深刻。代码审查是项单向活动,不用担心他们回过头来找你算账。所以还不如让“撕裂般的侮辱摧残他们的自信心”。他们也会期待着有一天,像你一样羞辱未来的初级开发者。

让审查没完没了

一点点代码审查算是“还不错”,那10小时的马拉松会是“非常棒”。请确保您审查了每一行代码,每一个配置文件的改变,每一个标记标签,每个自动重构。如果你曾把事情搞砸或者其他开发人员已经审查的代码,或者审查一个甚至没有写的代码的经验,一定会有加分。如果开发者不能确保把每个字符都加入到代码库中就惩罚他们,像博士毕业论文答辩一样严格要求。

保证所有审查必须“通过”

当你将情景设置为“教授和学生”,你不妨继续制定“审查必须通过”的政策。这样一来,它就不是一个专业团队检讨彼此的工作,并提出批评,建议和见解,而是一个被吓坏了的初级开发人员试图找出这次发布的版本中做的不够好的地方。为什么不能把每个开发/循环/发布搞得像参加SAT考试一样隆重呢?

以事实定位你的意见

只是你个人觉得静态方法比实例方法更好吗?当然不是。这是一个事实。想想你带有个人偏好的设计模式,编码方法,风格等的,然后去掉类似“我认为”,“我喜欢”,或者“在我看来”等修饰语,将“我喜欢用下划线命名字段名”变成“你应该用下划线命名你的字段名”和“我觉得你的参数很混乱”变成“我们的参数很混乱”。不管你做什么,不需要有任何证据或引用说明。当你告诉别人,他们的代码太长了,他们会问:“哪里太长了?”不用列举任何有关方法的长度和增加缺陷数成正比关系的研究,只消说“我是高级程序员,而你的代码比我长。”

逃避所有可以很容易地自动检查出的错误

为什么要花费一天时间坐在一个狭小的会议室,寻找是否有人犯了采用Camelcase命名而不是Pascalcase命名方法的大罪呢?你可以一行一行地数方法的代码行数,甚至通过手工计算循环的复杂度。当然,有很多工具可以在几秒钟内可以做任何你想做的事情,但这样就失去了公开指责初级开发人员错误的机会了。

保持消极

一点点积极的鼓励都会激励整个项目的进度,代码审查其实有很多这样的机会,因为你可以看到很多新的做事方法。正因为如此所以,你必须要小心,至始至终保持消极。使用白板或电子表格列出别人的错误,并且多多益善。隧道的尽头的灯只为懦夫照亮。

结语

我相信,良好的代码审查需要一致协作。如果发现了错误,你不需要直接告诉别人他们错了,换成“你觉得,如果我传递null给这个方法会怎么样呢?”就足够了,别人可能会说“哦,这个我没考虑到,我马上解决它。”让他们自己意识到错误并提出改进的方法会更好。

代码审查不是考试,也不是为了证明代码的价值,而是提出更好的解决方案的机会。通过结对编程来减少代码审查中的摩擦。这样一来,更多的人一起来捍卫自己的工作而不是一个团队来挑某个人的刺。

研究表明,减少缺陷数的最有效的方法不是过于苛刻,不是使用静态分析工具,甚至不是单元测试,而是结对检验。找一种工具来折腾是个不错的选择,只要像待人一样对待它就行了。

代码审查

在我们公司里要执行代码审查。这是我们每天的例行工作。事实上,今天的我们正是从这种一直坚持探索的漫长道路上走出来的。我们尝试各种技术、方法和工具,直到我们走到今天的成就(但这并不是说我们就此停步)。

在这旅途中,我们发现了很多的陷阱和危险,它们等待新手们上钩。这篇文章就是关于它们的:代码审查中的陷阱和误解。

代码控制:

很 多公司都把代码审查当成控制代码的方法。很多这样的公司都使用预提交策略。这种策略大多时候都是开源项目中使用,因为会有成百上千的提交者。可在一般的公 司里,很少会有这种情况。如果你雇用一个人,这意味这你要完全信任他,允许他将代码提交到代码库里。我知道有些公司会忍不住制定一些规程,要求程序员在提 交代码前必须进行“审查”和“批准”,但这并不能保证代码的质量。而且,程序员很快就会把这种代码审查当作一种“愚蠢”的公司形式过程,会开始抵制它(例 如,每月改一次密码。例如,使用像mypass1,mypass2等的密码)。

审判厅:

不 要把代码审查当成寻找替罪羊和追究责任的工具。比如说,这有一个错误,你找到“审查”这段代码的人,并责备他没有发现这个问题。这种事情会给公司里的开发 工作带来严重的影响。人们会挑出每个分号放置不正确的地方,因为他们担心会被当成替罪羊。团队成员开始缺乏信心,并最终失去互信。

责任任务:

不要过分要求程序员做代码审查。如果你强迫他们每天做一小时的代码审查,他们很快就会痛恨它,把它当成一种无趣的任务。代码审查是一种学习,是表扬,是获得反馈,是一种十分社交性的活动。代码审查应该是有趣的,不要让它变的无聊。

我和我的代码

如 果你的代码被某人审查,他会留下一些注释(有时是不那么友好的话),不要生气。他并不是在说你是一个很烂的程序员。这不是他的本意,也不是代码审查的目 的。他的所作所为是在批评代码(而不是作者)。代码审查是针对代码,不是针对你。不要把代码审查当成互相讽刺的论坛和相互批判的工具。当你写审查注释时, 努力保持不要粗鲁,也不要太苛刻。努力站在作者的立场上看待这些代码。