怎样做一个合格的 CodeReview
引子
CodeReview 是一件很需要耐心和技术功底的活动。
-
不了解业务，很容易沦为代码风格检查；
-
代码量比较大，很容易走马观花，放过业务细节上的纰漏；
-
技术功底不够，很容易放过性能或安全上有漏洞的代码；
-
代码细节不严格追究，似乎达不到 CR 的力度；严格追究，来回沟通好几趟。
-
需要设定标准和底线。底线达不到，坚决不通过，不因任何问题而妥协。
最难的是，CodeReview 是一项需要全局观的活动。哪怕仅有一行代码改动，也需要在更大的业务语境里去审查其是否恰当；如果有多行改动，恐怕要结合该业务的所有代码去分析。因为，改动的代码有可能单看上去没有问题，但结合已有代码及逻辑，就可能潜藏错误或漏洞。
BUG 无处不在。以人类现有的理解力和审查力，基本不可能避免大型软件里的 BUG 的产生（你无法看到看不到的 BUG）。能够做到的是： 限制 BUG 产生的影响使之微乎其微。
必要前提
-
阅读 CodeReview 相关的技术文档及说明，熟悉相关业务逻辑及改动意图；
-
代码提交者保证遵循必要的代码规范和风格；有极少量违反可以提醒，大量违反直接打回；
-
代码提交者保证提交之前编译通过，单测全部通过；
-
代码提交者保证功能的自测通过；
-
代码提交者自己先行 review 一遍，纠正低级错误；
-
CodeReview 重点检查必要规范、关键逻辑、质量层面。
两个敏感素质
无论技术水平如何，业务熟悉度如何，CodeReview 的人应当具备两个基本敏感素质：
-
影响面评估敏感。 对代码涉及的影响面敏感，即使只有一行代码；
-
对代码潜在问题敏感。看到一行代码，能敏锐意识到可能存在的问题：NPE、健壮性、性能、可维护性、安全等。
常用检查项
规范与风格
-
命名内容： 是否清晰直观表达意图；是否有粗略不贴切的问题；是否与项目约定一致；
-
命名风格： 变量/方法名 - 驼峰式；常量 - 大写+下划线；（新手容易犯）
-
注释： DO 属性是否有加业务含义注释；（建立团队约定）
-
魔数： 使用常量定义；（情不自禁都会犯，要坚持零容忍）
-
拼写错误： 要杜绝拼写错误，尤其是两个字符对调的情形（零容忍）；
逻辑层面
-
参数校验： 参数格式、参数内容、参数合法性校验（必须）；
-
异常：异常是否被捕获并适当处理；（必须）
-
关键代码： 要细读，反复推敲；（必须）
-
模板代码： 是否有拼写错误；（必须）
-
算法：是否有详细说明、引用出处；（推荐）
-
流程：复杂流程是否有相应文档说明；（推荐）
-
注释：不明显的逻辑、workaround 方案、临时方案、很难用简单单词表达的复杂业务含义等要加注释说明（建议）；
表达层面
-
拥挤的代码：多行挤在一行最好能分拆为多行；（推荐）
-
嵌套 if-else: 是否能够解开打平；是否可使用策略模式来分离；（推荐）
-
费解的代码：理解起来有点绕弯，不够直观清晰地表达意图；（建议）
-
建议的方式：可以给出更好的代码实现建议；（建议）
质量层面
-
NPE： NPE 无处不在，要严加防范；API 调用、数据库查询、反射获取的对象、多级嵌套对象等返回的对象务必要做判空检查（必须）；
-
健壮性： 主要检查越界、字符串解析异常；split, indexOf 等方法要注意越界异常；Json 解析要注意非法格式及非法数据校验（必须）；
-
重复代码： 重复代码，是否可以抽离子函数或模板处理（重复代码敏感）；
-
可维护： 散落在流程里的领域对象行为；过长的参数；过长的方法；方法内部修改入参等行为；
-
统一性： 同一个概念，是否创建了多个不同类来重复表达；
-
单测：是否有必要的单测；单测是否覆盖主要逻辑；
-
性能：是否有循环调用 API 或访问数据库的做法； 是否面临大数据量场景（是否有加索引措施、SQL 编写是否适当等）；是否需要加缓存（本地还是分布式）；
-
并发： 是否有多线程访问的可能性；是否有不受控的线程或线程池创建；是否有加同步访问；同步手段是否恰当；并发性能是否能够接；是否存在死锁风险；
-
安全： 越权访问；敏感信息明文；SQL 注入； 动态代码执行是否有恶意参数校验；
-
日志： 是否有必要的日志信息（info, warn, error）；是否简洁得当；异常是否捕获并仔细打印了业务ID 相关的日志；
方法
-
可以先初步过一遍，检查下是否明显有以上问题，及时反馈；
-
进一步再深入业务核心，去审查业务逻辑是否有问题；
-
将模板代码与关键代码区分开；
-
关键代码要仔细研读；
-
可以在开发的停顿间隔之间进行；
-
建议至少在 IDE 里审查一下改动，在 IDE 里看更有感觉，在页面上看由于缺失上下文语境，容易遗漏；
-
把程序运行起来，亲自试一试，或许你会有一些和他们测试时不同的操作，发现一些他们遗漏的问题。
建议
-
对于一些比较紧急的改动会留下改进建议，但可以通过，通过后续代码提交解决遗留的问题；
-
可以对Review的评论进行分级，不同级别的结果可以打上不同的Tag，比如[required]必须修改，[recommend] 推荐用法，[suggestion] 可选建议，[question]有疑问需要澄清；
-
可以考虑团队统一安装某个代码检查插件，提交之前先解决插件指出的问题，减少 CR 成本；
-
比较大的改动，可以分批提交，比如分成多个分支；
-
评论宜正面积极，给予建议而不是指责；
-
对于新手，更多是给予代码规范上的建议；对于高级工程师，更多给出业务和质量层面的提醒；
-
不要吝啬你的赞扬；好的代码也有必要拿出来让大家一起学习。
