代码审查要点

代码审查| 2016-07-07

1. 做代码 Review 的人的责任

给别人的代码做 Review 的人(Reviewer), 他的责任不仅在于保证代码质量, 更重要地是承担拼盘者角色. 当编写代码的人未来因忙于别的项目, 休假或者别的原因无法在岗时, Reviewer 将负责接手项目, 修复 bug, 增加新功能等. 所以, Reviewer 必须为自己着想, 认真负责地进行代码 Review, 以免日后自己接手一个逻辑不通, 代码混乱的项目(程序员俗话说的”一坨屎”).

2. 做代码 Review 的原则性要求

首先, 做代码 Review 前, 要了解产品需求, 并从产品需求出发, 整理自己的技术思路, 然后讨论被 Review 者的逻辑, 看看是否有逻辑漏洞. 如果有逻辑漏洞, 那么必须全部打回, 因为逻辑一旦不准确不严谨, 后面的代码就是在建造 bug 而不是开发功能.

3. 代码封装

代码封装的思路必须从纯朴出发, 不能在第一个版本就过度设计和过度封装, 不必做过多的抽象. 例如, 代码要尽量展开, 代码逻辑一目了然, 串行化直肠子, 不要跳跃, 不要回调, 只要纯朴的过程式编程, 不要发明各种解析和组装机制, 让代码和逻辑尽可能一一对应.

直观和一目了然, 一般是可以很好解决问题的. 但随着时间和业务的发展 — 注意, 这是一个过程, 这个过程不能反过来否定纯朴 — 会导致代码重复, 类函数过长, 这时可以开始进行函数封装, 或者抽象出新的类来组织代码.

封装和目的至少有一半是为了组织代码, 而不是为了实现设计. 所以, 先不封装, 然后再封装, 般到桥头自然直, 提前量打得太多的话就变成赵本山卖拐了.

4. 代码的放置

我们写的代码都不是教科书上那种简单的十几二十行代码的功能, 而是多个类, 多个文件, 多个函数交叉的大量代码, 所以代码放在哪个文件, 哪一行都是有讲究的.

原则上, 解决同一类问题的代码应该就近放在一块, 空间上不要距离太远, 中间不要隔着别的逻辑. 对于 Web 应用来说, 当用户浏览器访问时会执行一段代码处理某类问题, 同时, 异常执行的代码(如 crontab)也会去解决此类问题, 那么, 应该把解决此类问题的代码封装成一个函数或者类, 然后在 Web 处理请求时调用和在 crontab 执行时调用, 而不是让代码出现在两个地方, 即使是这两个地方处理的是同一个问题的不同部分, 也应该封装起来, 放置到一块.

5. 注释

代码应该有一定比例的注释, 特别是业务型代码. 由于业务的多样性, 当遇到业务逻辑不符合常理, 或者某些临时解决方案, 某些照顾特殊, 等等不符合普世公认的常理, 但有合理理由存在的需求时, 你必须在代码中进行注释, 这时, 在注释中描述业务的不合理之处, 然后说明代码是如何就对的.

所以, 代码中必须有注释, 从自我的角度考虑, 要注释那些可能变化的东西, 以免自己过两天连自己都不认识了. 从 Reviewer 的角度考虑, 如果你不要求别人写上注释, 等你接手之时, 倒霉的是你自己, 而不是别人.

另外, 注释可以解释代码的逻辑. “1,2,3,4” 这样一条一条按顺序地列出来, 一一对应后面的代码逻辑块. 如果你从来没有写过”1,2,3,4″这样的注释, 说明要么你不懂得封装的好处, 要么就是过度封装而丧失了过程式编程技能. 任何一个再高级的业务逻辑, 总是能找出最核心的”1,2,3,4″串行逻辑, 那些跳跃的类和函数的交叉调用, 都是为了最终的”1,2,3,4″而打造的周边, 不是核心, 核心是过程式的, 是串行化的.

再回到代码封装和放置, 最核心的”1,2,3,4″那段串行逻辑, 一般不要再被打断分散到不同的地方, 而应该让他们在文件里连续出现, 一屏就能全部显示, 出没出问题有没有漏洞一目了然.