王淮是 Facebook 的早期员工,也是 Facebook 内部第二位中国籍工程师和第一位研发经理,曾经负责支付后台和安全系统,担任反欺诈部门的技术经理。最近,他分享了一些基本的码农原则,包括代码结构、测试重要性等。
正确性是第一要求,他认为“不能解决问题的代码是耍流氓”。
然后是代码结构,王淮觉得结构体现逻辑:
第一步,第二步,需要什么数据,需要做什么处理,处理完了结果到那里去,都应该在结构中被很好的体现出来。
结构体现设计。 设计一定要清晰。我的经验来看,一般来说,在设计结构图上面的每个组件都对应着自己的类,然后之间或类内部的通信通过成员函数来完成。
他提供一个可借鉴的做法:在一个大的功能实现过程中,给出第一个 diff(提交代码增量)的时候,可以只把结构当做一个 diff,里面的函数可以是空的 (place holder,作为占位符)。 把数据的生成和界面的展示分开来。典型的可以按照 MVC 的模式来,但也可以只把数据和 UI 界面分开来的比较轻量级的做法。结构应当是提交代码审查的时候最最关注的地方。最需要问的问题就是“这个 diff 号称要解决的问题被正确解决了吗?”
测试的重要性不言而喻。王淮指出:
不论你再正确,还是有错误的时候。通过(相对)公正的测试一是来减少自己被绕到代码里的几率,二是让后续的或者别人的改动对自己代码不经意的破坏被最快的展现出来。测试应该把类主要的功能都测一遍。测试也应该把类和其他类最重要的集成部分也测一遍。
对于提交代码增量的问题,王淮说了几个需要注意的事项:
- Bug 修改,无所谓,该多大就多大。一般 bug fix 不会超过 100 行。超过的要特别重视,想想究竟是什么原因造成。会不会是当初设计的问题。一个 diff,原则上不应该超过 200-300 行修改。但多了怎么办,把一个 diff 变成多个。
- 每个 diff 应该只做一件事情。每个 diff 尽可少的做一个改动。这样可以尽可能的方便自己的管理 (学会用 git branch),和方便审阅者的代码审查。如果 diff 越集中做一件事,审查代码的人需要越短的时间来审查做出高质量的,整体效率越高。
- 一个功能超过一屏,则将其分割。
面向对象还是面向函数,王淮指出,Java 本身就是面向对象,所以这个问题不大。但千万不要出现披着面向对象的外皮,在类里面写超长的面向函数的处理。这种情况下,尽可能的分流成辅助函数。
关于代码规范,他举了一些简单的例子:比如文件名,变量或函数名的命名规范,分行的前置空 2 个空格或 4 个;每行的字数(不应超过 80 字符);如何使用 public/private/protected;用左右括号的原则;空行的使用;文件和代码 comments 的位置 (比如,代码注释只能单独成行);对“// TODO:”的使用规范;宏、常量的使用。这里没有特别的哪一种样式一定更对,但是需要有一个大家统一的指南,一起遵守,让整体的代码有统一的风格和标准。最大的好处就是有利于可读性,可读性的好坏决定着维护成本。
注释应当简洁但充分。有些人觉得代码应该自说名。王淮不大同意,代码是实现细节,适当的在意图上给予说明,可以大幅度的减少读代码的人的烦恼。
代码审查越来越受到重视,王淮指出:
- 作为审查者,一定要读懂 diff;所有被接受的 diff 必须是在读懂的前提下。做审查者的人要有“将来如果这些代码线上出问题,我要帮助支持”的心理准备。
- 代码审查应该被每个工程师当做工作的重要一部分。
- 应当在 24 小时内给回复,这应当变成共识。
- 感觉有问题的代码,一定要在相应的行上做出评论 (inline comments),以让作者明白问题所在。
- 尽可能把对修改的所有意见一次性给出,减少来来回回的次数。比较复杂的建议审查者主动找代码作者来进行线下沟通,达成一致。
- 一般的 diff,来回次数不宜超过 3 次;如果超过 3 次,想想看,是不是 diff 太大,太复杂了。
说到代码审查,thoughtbot 公司最近在 github 上发布了自己的代码审查原则,其中几个值得关注的要点包括:
- 接受这样的现实:很多编程上的主张都是一种个人观点。应该讨论它们的利与弊,提出你的倾向观点,迅速地达成一种解决方案。
- 提问,而不是命令。(“把这个变量改成
:user_id,
你觉得如何?”) - 请求给予说明。(“我不明白。你能解释一下吗?”)
- 避免代码的归属之争。(“我的”,“不是我的”,“你的”)
- 避免使用一些会被认为是有关人身特征的词语。(“笨蛋”,“愚蠢”) 要把所有人都看作是有魅力的、聪明的、善意的。
- 要明确。要记着并不是每个人都能理解你的意图。
- 要谦虚。(“我不能确定——我们来分析一下。”)
- 不要用夸张修辞语。(“总是”,“从不”,“永远”,“毫无…”)
- 不要讽刺。
- 展现真实的你。如果你不是幽默型的人,不喜欢使用一些表情符号或动画 gif 图,不要勉强。如果你是这种人,请自信的发挥。
- 如果有太多的“我不理解”或“另一种方案:”的评论,请专门针对这个人进行交流。可以把你们线下的交流总结成一个帖子附在后面。
- 对审查者的建议表示感激。(“谢谢提醒。我会把它改正。”)
- 理解审查是对事不对人。审查的是你的代码,而不是你。
- 解释为什么代码写成这样。(“因为 xxx 原因我才写成这样。如果我把这个类 / 文件 / 方法 / 变量改个名会更清晰些吗?”)
- 整理所作的改动,在以后的迭代中重构它们。
- 在做修改的版本上注明代码审查的链接。
- push 提交要基于最早的一轮反馈,并形成一个独立的分支。等这个分支上的任务完全完成了再合并。这让审查者能够根据早先的反馈找到你的单独的更新。
- 努力站在审查者的立场上理解。
- 争取回复每个评论。
- 直到最后一个人退出登录后再合并分支。
- 直到持续集成测试 (TDDium, TravisCI, 等) 告诉你这个分支的测试套件通过后再合并分支。
评论