|
|
这篇文章只是体现我以前写代码和做代码审查时候的一些原则。供大家借鉴。欢迎大家补充。
正确性 (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)
郑重声明：本文版权归原作者所有，转载文章仅为传播更多信息之目的，如作者信息标记有误，请第一时间联系我们修改或删除，多谢。
