Airbnb女神工程师朱赟:什么才是Code Review的正确姿势?

分享到:  QQ好友和群QQ好友和群 QQ空间QQ空间 腾讯微博腾讯微博 腾讯朋友腾讯朋友 微信微信
查看查看281 回复回复4 收藏收藏 分享淘帖 转播转播 分享分享 微信
查看: 281|回复: 4
收起左侧

Airbnb女神工程师朱赟:什么才是Code Review的正确姿势?

[复制链接]
小开开 发表于 2016-8-16 04:14:36 | 显示全部楼层 |阅读模式
快来登录
获取最新的苹果动态资讯
收藏热门的iOS等技术干货
拷贝下载Swift Demo源代码

之前写过一篇《写代码的四个境界》,那个时候,大部分时候我还是愉快地写着自己的代码。Code review 也是每天工作的一部分,但是相对而言花的时间还是有限的。

最近一是因为角色转换,二是突然来了很多新人。花在 code review 上的时间比写代码多出了好多,也有一些心得和感触,随便写写吧。

Airbnb女神工程师朱赟:什么才是Code Review的正确姿势?

Airbnb女神工程师朱赟:什么才是Code Review的正确姿势? - 敏捷大拇指 - Airbnb女神工程师朱赟:什么才是Code Review的正确姿势?


总的说来,硅谷稍具规模的公司 code review 的流程都是比较规范的。模式也差不多。一来所有的 PR 都必须有至少一个人 stamp,才能 merge。如果改的东西涉及到多个项目,则需要每个项目都有人 stamp 才行。还有一些特别关键的代码,比如支付相关的,通常也会需要支付组的人 stamp 才行。

NOTE: PR (pull request, a set of changes someone tries to push to a GitHub repository. Once a PR is sent, interested parties (peer engineers) can review the set of changes, discuss potential modifications, and even push follow-up commits if necessary.

在 stamp 前,通常是 github page 上可以给出各种 comments,page 是共享的,所以谁都能看到。有些 comments 是询问,那么代码作者直接回复或解释就行。有些 comments 是指出代码的问题,这样代码作者可能会依此改动;也可能不同意,那就回复 comments,有的时候关于一个实现细节,讨论的 comments thread 可以多达十几条。这样对于大家达成共识很有帮助。

以前遇到有朋友说:

“看到代码写的太烂,觉得来回扯皮效率不高,干脆扑上去自己写。”


曾经我也有过类似的想法。不过最近遇到的一些事让我慢慢地意识到这种想法很要不得。

首先就是从对方的角度来说。对方写不好,可能是对业务不熟悉,可能是对语言不熟悉,也可能是对公司代码的大架构不熟悉。如果你是帮他/她 “写” 而不是耐心指出哪里有问题,那么下一次,他/她可能还是不知道。不仅无益于别的人成长,有的时候甚至会让别人有挫败感。

再然后就是这种方式的可扩展性很差。即使 code review 会花掉哪怕十倍于你自己写的时间和精力,但它会让人明白代码应该怎么写,从长远来看,这其实是在一定程度上 “复制” 你的生产力。你不能什么都自己写。尤其是你慢慢开始带项目、带新人。每天 review 五个人的代码,和写五个人的代码,你觉得长期而言哪个更合算?

此外,如果说写代码是一个学习过程,怎么做一个好的代码审核人更是一个学习和成长的过程。自己绕过一个坑不难,难的是看到别人那么走,远远地你就能告诉他/她那里有个坑。而他/她在经你指出多次后,下一次他/她也会帮着指出别人的类似的问题。

这一点最近感触尤为深刻。前一阵组里咔咔咔接二连三来了好多新人,老大说,你少写点代码,多做点 review。于是那几天我几乎工作的一半时间都用来看代码,写 comments。可是最近就会发现,他/她们相互之间大部分时候已经可以很好的相互 review 了,于是我又腾出好一些时间来做别的工作。

Code review 做多了,发现其实也是有一定套路的,试着归纳如下:

  • 代码格式方面。很多公司都有 coding style guideline。大家的约定俗成,避免公司的代码风格不一致,也避免一些不不要的为了 “要不要把闭括号另起一行” 而无谓地争论,除非是不小心,通常大家都不会弄错。但是新员工往往会在这方面还不太熟悉。这一类问题也比较容易指出。
  • 代码可读性方面。这包括一个函数不要太长,太长就 break down。所有的变量名尽量能够说明它的用意和类型。比如 hosting_address_hash,一看就知道是房东地址,而且是个 hash 类型。不要有嵌套太多层的条件语句或者循环语句。不要有一个太长的 boolean 判断语句。如果一个函数,别人需要看你的长篇注释才能明白,那这个函数就一定有重构的空间。另外,如果不可避免有一些注释,则一定要保证注释准确且与代码完全一致。
  • 帮代码作者想想他/她有没有漏掉任何 corner case。很多时候这是业务逻辑相关的,尤其需要比较老的工程师帮助指出需要处理的所有情况。
  • Error handling。这是最常见也是代码审核最容易帮别人看出的问题。举个例子,下面一段简单到不能再简单的代码就至少有三个潜在的问题:params 里面需要 validate 是不是有 user_id 和 new_name 这两个 key;能不能找到这个 user_id 对应的 user;save 的时候会不会有 DB level 的 exception,应该怎么处理。


[Delphi] 纯文本查看 复制代码
def update_user_name(params)
	user = User.find(params[:user_id])
	user.name = params[:new_name]
	user.save!
end


  • 测试例和防坑。测试例不用说了,每段代码都应该有测试例。但是对于一些你能预见如果别人改动代码会引起可能问题的情况,一定要额外的加测试例防止这种事情的发生。这一点没有例子参考也不太好说。怎么写好测试例,本身就值得写一篇文章了。
  • 小架构。什么意思呢,就是一个文件或者类内部的代码组织。比如如果有重复的代码段,就应该提取出来公用。不要在代码里随意设常数,所有的常数都应该文件顶部统一定义。哪些应该是 private,等等
  • 大架构。这个就更广了。包括文件组织,函数是不是应该抽象到 lib 或者 helper 文件里;是不是应该使用继承类;是不是和整个代码库的风格一致,等等。


当然,视具体情况可能还有很多别的需要在 code review 中注意的。但是如果按照这个 checklist 过一遍,一些明显的问题就可以避免个八九不离十了。

另外,代码 review 和写代码是我们每个工程师都应该致力的两个方面,缺一不可。可能在工作中的某个阶段或者某个项目里,你会花更多的时间在某一面,但长久看来,两个技能缺一不可。

代码审核是工作,不要抱有情绪化。我曾经干过一件事,一个外组同事的代码,别人已经 stamp 了,可是我觉得有问题,于是把 stamp revert 掉了,在 comments 里写了为什么有问题,和建议的改法。当时心里还有点觉得好像怪得罪人的。可是后来发现同事反而很客气地接受并道谢了。心里也是很感激有这样的工作环境。

另外,经常会遇到有些 review 的 comments 里出现不同意见,其实是两个人在编程习惯和约定俗成上没有共识。如果在不违背公司 style guideline 的情况下,没必要一定让对方和你有一样的习惯。如果你真的觉得这样更好,通常我也只会写 “I personally would prefer A over B, but no strong opinion.” 或者 “How about change it to A, since ... However, this is not a merge blocker. ”

人都有不周到的时候。多几双眼睛一起帮你看一遍,就可以很大程度地减少代码里的错误。另外,相互 review 的过程中还能从彼此那里学到很多编程的小技巧和好习惯。细想来,很多 Java 和 Ruby 的特别好用的 feature,我都是在 code review 的过程中学到的。

参考阅读:《写代码的四个境界




相关内容

AirBnb女神工程师视角:以创业者的姿态做一名工程师

Airbnb工程师朱赟Angela杂谈API的基本原则、框架、设计

Airbnb工程师朱赟:漫谈产品的国际化和本地化,互联网的本质是 “人生人”?!

Airbnb女神工程师朱赟:写代码的四个境界

Airbnb女神工程师朱赟:什么才是Code Review的正确姿势?




作者:朱赟(Angela),美国硅谷 Airbnb 的资深工程师。原题目《说说 Code Review》,微信公众号“嘀嗒嘀嗒”(ID:AngelaTalk)。

都看到这里了,就把这篇资料推荐给您的好朋友吧,让他们也感受一下。

回帖是一种美德,也是对楼主发帖的尊重和支持。

*声明:敏捷大拇指是全球最大的Swift开发者社区、苹果粉丝家园、智能移动门户,所载内容仅限于传递更多最新信息,并不意味赞同其观点或证实其描述;内容仅供参考,并非绝对正确的建议。本站不对上述信息的真实性、合法性、完整性做出保证;转载请注明来源并加上本站链接,敏捷大拇指将保留所有法律权益。如有疑问或建议,邮件至marketing@swifthumb.com

*联系:微信公众平台:“swifthumb” / 腾讯微博:@swifthumb / 新浪微博:@swifthumb / 官方QQ一群:343549891(满) / 官方QQ二群:245285613 ,需要报上用户名才会被同意进群,请先注册敏捷大拇指

嗯,不错!期待更多好内容,支持一把:
支持敏捷大拇指,用支付宝支付10.24元 支持敏捷大拇指,用微信支付10.24元

评分

参与人数 1金钱 +10 贡献 +10 专家分 +10 收起 理由
Anewczs + 10 + 10 + 10 32个赞!专家给力!

查看全部评分

本帖被以下淘专辑推荐:

昏戏湿 发表于 2016-8-16 18:40:26 | 显示全部楼层
“复制” 你的生产力!
 楼主| 小开开 发表于 2016-8-17 21:44:21 | 显示全部楼层
苏格拉没有底 发表于 2016-8-18 01:20:11 | 显示全部楼层
corner case,思维的严谨严密
苏格拉没有底 发表于 2016-8-18 01:27:50 | 显示全部楼层
您需要登录后才可以回帖 登录 | 立即注册

本版积分规则

做任务,领红包。
我要发帖

分享扩散

都看到这里了,就把这资料推荐给您的好朋友吧,让他们也感受一下。
您的每一位朋友访问此永久链接后,您都将获得相应的金钱积分奖励
热门推荐

合作伙伴

Swift小苹果

  • 北京治世天下科技有限公司
  • ©2014-2016 敏捷大拇指
  • 京ICP备14029482号
  • Powered by Discuz! X3.1 Licensed
  • swifthumb Wechat Code
  •   
快速回复 返回顶部 返回列表