我的码农原则
这篇文章只是体现我以前写代码和做代码审查时候的一些原则。供大家借鉴:1、正确性,不能解决问题的代码都是耍流氓;2、可读性, 统一的代码规范;diff 发出去之前,code-review 之中,check-in 之前分别应该做什么?
@王淮
这篇文章只是体现我以前写代码和做代码审查时候的一些原则。供大家借鉴。欢迎大家补充。
正确性 (Correctness)
正确性是第一要求。不能解决问题的代码是耍流氓。
- 结构 (Code Structure)
结构体现逻辑。第一步,第二步;需要什么数据,需要做什么处理,处理完了结果到那里去,都应该在结构中被很好的体现出来。
结构体现设计。 设计一定要清晰。我的经验来看,一般来说,在 design chart 上面的每个 component 都对应着自己的 class,然后之间或 class 内部的通信通过 member function 来完成。
一个可借鉴的做法 – 在一个大的 feature implementation 过程中,给出第一个 diff 的时候,可以只把结构当做一个 diff,里面的函数可以是空的(place holder)。
把数据的生成和界面的展示分开来。典型的可以按照 MVC 的模式来,但也可以只把数据和 UI 分开来的比较轻量级的做法。结构应当是 diff review 时候最最关注的地方。最需要问的问题就是“这个 diff 号称要解决的问题被正确解决了吗?”
- test 的重要性
不论你再正确,还是有错误的时候。通过(相对)公证的 test 来 1)减少自己被绕到代码里的几率;2)让后续的或者别人的改动对自己代码不经意的破坏被最快的展现出来。
test 应该把 class 主要的 function 都测一遍
test 也应该把 class 和其他 class 最重要的 integration 也测一遍。
可读性 (Readability)
Readability leads to maintanence cost.
- diff 的大小
bug 修改,无所谓,该多大多大。一般 bug fix 不会超过 100 行。超过的要特别重视,想想究竟是什么原因造成。会不会是当初设计的问题。
一个 diff,原则上不应该超过 200-300 行修改。但多了怎么办,把一个 diff 变成多个 – split to multiple changes.
- 每个 diff 应该只做一件事情
每个 diff 尽可少的做一个改动。这样可以尽可能的方便自己的管理(学会用 git branch),和方便 reviewer 的代码审查。如果 diff 越集中做一件事,审查代码的人需要越短的时间来审查做出高质量的,整体效率越高。
- 一个 function 超过 1 屏 => split it, idiot.
-
- 统一的代码规范
比如文件名,变量或函数名的命名规范,分行的前置空 2 个 spaces 或 4 个;每行的字数(不应超过 80char);如何使用 public/private/protected;用左右括号的原则;空行的使用;文件和代码 comments 的位置 (比如,代码 comment 只能单独成行);对“// TODO:”的使用规范;macro,constant 的使用;
等等等等。
这里没有特别的哪一种 style 一定更对,但是需要有一个大家统一的 guideline,一起遵守,让整体的代码有统一的风格和标准。
最大的好处就是有利于 readability.
- object-oriented v.s function-oriented
Java 本身就是面向对象,所以这个问题不大。但千万不要出现披着面向对象的外皮,在 class 里面写超长的面向函数的处理。这种情况下,尽可能的分流成 helper function.
- crispy & sufficient 的注释
注释应当简洁但充分。有些人觉得代码应该 speak for itself。我不大同意,代码是实现细节,适当的在意图上给予说明,可以大幅度的减少读代码的人的烦恼。
diff 发出去之前
- 与 master 做一次 merge update,确保 resolve all conflicts
- run 一次所有涉及的 test cases (需要工具)
- 考虑最可能做 reviewer 的人,可以是团队伙伴,也可以是修改涉及到的源代码的 owner。但必须是关心该改动或和改动相关的人。
- 所有的 manager 应当自动 subscribe 自己的团队里所有人的 diff requests (做好 filtering,但你不一定要看)
code-review 之中应该做的
- 作为 reviewer,一定要读懂 diff;所有被 accept 的 diff 必须是在读懂的前提下。做 reviewer 的人要有“将来如果这些代码线上出问题,我要帮助 support”的心理准备。
- code review 应该被每个 engineer 当做工作的重要一部分。做 Performance Review 的时候应该把帮助做过的 code review 考虑,for both employee & manager.
- 应当在 24 小时内给回复,这应当变成共识。
- 感觉有问题的代码,一定要在相应的行上做出评论 (inline comments),以让作者明白问题所在。
- 尽可能把对修改的所有意见一次性给出,减少来来回回的次数。比较复杂的建议 reviewer 主动找 author 来进行线下沟通,达成一致。
- 一般的 diff,来回次数不宜超过 3 次;如果超过 3 次,想想看,是不是 diff 太大,太复杂了。
check-in 之前应该做的
- 与 master 做一次 merge update,确保没有问题
- run 一次 code change 涉及到的所有 test cases(包括新增的和涉及到的 test cases)来自: www.tmtpost.com