代码质量检查
Review 流程
写完代码后,发送 diff 给指定人 review。待给出意见后,修改代码,继续提交 diff,直到 review 通过。通过 review 的代码由审阅者自动或者手工 Merge 代码到指定分支。多人协作 review 时,第一位阅读者给出粗略意见,第二位看意见是否给的恰当,再补充其他意见。多人同时 review,也可将代码用投影仪打到大屏,共同 review 一段代码。
这里需要思考几个问题:
- review 的分工。有多人参与时需要考虑这个问题。最好有一个总负责人。
- review 的策略。项目初期应该主要考察代码结构是否符合规范,即代码是否放到了相应的位置。项目中期应该以逻辑正确、简化代码、重构现有代码为主要考察点。项目后期主要看程序性能相关的优化问题。
- review 通过的策略。 什么样的代码才能通过 review。 是不是需要完全符合 checklist? 我个人并不赞同完全遵照 checklist。后面我会细说。
- 如何反馈已修改?review 代码的工具一般是 gitlab, reviewboard, gerrit。这些工具都提供了 comments 这一功能。review 的时候可以写下 comments。如果对照 comments 进行修改,那么在 comments 后回复已经修改即可。
- 如何反馈拒绝?这个最好找审阅者当面说清楚:为什么不按照建议编写代码。如果认为自己文笔非凡的也可以在 review 工具中写明白。
- review 修改后再次 review 的策略。要对照之前的 comments 看看编码者是如何修改的。当然也要看看有什么新加入的代码。
- 代码没有必要写完后再 review。可以写一部分 review 一部分。
阅读代码的技巧
如何阅读别人提交的代码?首先要了解这代码的上下文和需求,不能不负责的指点江山。要充分了解需求,才能考虑如何实现,然后才能审阅别人的代码。不能因为读不懂别人的代码就拒绝别人的代码。这事开源界常发生,但公司内部最好杜绝发生。大家都是同事,低头不见抬头见,没必要关系僵化。你是能炒别人鱿鱼还是你自己要离职呢?看不懂代码可能有三种:一种使用了混淆、一种提交者写的烂、一种审查者能力不足。
混淆代码一般都是有特殊原因的,但不会是故意刁难审阅者。比如说机密的模块、需要隐藏秘钥或算法。这部分代码讲明白意思即可,一般也不需要 review。
如果提交者写的太烂,一般都是 i, j, k 的命名导致。审阅者更应该读懂后,指出哪些地方需要改进:比如少写了注释,改个好名字等。写的烂并不会导致你读不懂,一般是你不愿意花时间去读。如果你说事不关己,没必要看烂的代码。这种心态也不适合进行 Code Review,只适合小作坊编程。不如谦虚谨慎、虚怀若谷。
而最后一个原因:请先提高自身的编程能力!有许多算法精妙难懂,在自己编程水平不足的时候决不能以读不懂为由拒掉。
当你有足够的编程能力时,review 代码时请遵照下面的流程:
粗读(知道大概写了哪些内容)
=> 抽取重点(架构/类的定义/代码结构)
=> 细读代码(简化/重构)
=> 逻辑判断 (各分支)
=> 细节优化(资源释放,线程安全、算法效率)
-
通过粗读,我们需知提交者改动了什么。是大量修改?还是大量新增?有没有针对性的测试?没有改动配置文件? 有没有改动数据库?
-
我们再仔细看看设计是否恰好符合了需求。是顺序的代码,用到了多线程?还是使用了设计模式?抑或用了第三方组件/架构?是集中式,还是分布式?数据是怎么存储的?单元测试有没有测重点?还有没有更好的设计?
-
再细读一遍代码。挑出繁琐的代码与相似的代码。看是否有简化或重构的可能。嵌套是否太深。是否有难懂的代码。
-
仔细寻找逻辑判读的语句 if,while,for 等。主要看每一个分支是否符合需求,是否是笔误。
-
最后把你的注意力集中在资源释放、线程安全、算法效率上。看看有没有优化的空间。
- 参考文档 Code Review 生存手册