# 聊聊我是如何进行Code Review的
背景：入职公司以来，就觉得老代码的代码写得非常放飞，尽管有些微服务的代码量不大，但是改动成本极高。后面我们部门对一些关键的领域进行了重构，代码质量提高了很多。但是自从公司的技术人员扩招以后，架构、代码质量开始腐烂，虽说没刚入职的时候看见的代码差，但以这进度下去，迟早又得重来一次推倒重来。为了避免再一次大重构，总结了一些我平时Code Reiew比较注意的点。
Code Review的好处就不赘述了，主要就是：防止架构、代码腐化；旁观者清，提前发现缺陷；检查是否和技术方案一致；统一团队的编码风格；防止代码单点维护问题。
# 谁来Code Review？
一般是leader，领域、微服务的负责人，也可以是次要负责人、结对编程的partner。
# 什么时候进行Code Review?
应该结合团队、项目情况进行Code Review，需要带着目的，而不是按部就班的进行。
- 日常Code Reiew。我一般是上线日等发版时，Review之前没Review过的代码，一般这些都不是关键的代码。
- 需求Code Review。由于资源有限，需求Code Review我只会看重要的领域、初级工程师的代码。一般是项目开发进入里程碑，代码经过单元测试、集成测试，就可以开始Review了。
- 重点代码Code Review。重要领域，除了需求上线，在Bug修复、租户需要配置变更时，也需要进行Code Review、配置Review，以防万一。
- 上线Code Review、配置Review。上线前，对比之前需求Code Review的版本的变更，看变更了哪些内容，一般这节点的Review所需要花的时间很少。
# Code Review一般主要关注哪些内容？
代码规范不赘述了，一般都是基于阿里的代码规约，再加上公司的实际情况定出来的，我们也不例外。
# 可读性、可维护性
- 命名。命名应与需求文档、技术方案文档一致，不使用生僻词、少使用拼音。
- 不使用魔法值，不混用常量。
- POJO使用驼峰，贴@JsonNaming注解。团队规约。
- 方法不超过50行、类不超过2000行。太长则不利于阅读、可能说明职责不明确。
- 参数个数不超过5个，以上使用包装对象传输参数。太长容易传错参数。
- 注释包含相关文本资料。比如：改动应包含改动的原因；相关需求；bug的工单地址链接；外部系统接口的文档地址、位置；
- 字段尽量使用@see注解引用相关常量类。方便其他成员查看代码。
- 尽量不用JsonObject、Map等动态容器处理业务逻辑。除了编写者，很难看懂其中的逻辑和数据流向，毕竟我们都是面向IDEA编程的。
- 代码、逻辑是否重复，是否可抽取出来。一般可以这么做：同一个类中的重复代码抽成一个方法；同一个父类的子类的重复代码抽象到父类，成为模板方法或者protected的方法；无继承关系的类的重复代码抽到一个第三方的类，它可能是一个工具类、服务类、BO类等等。
- 外部系统接口是否使用防腐层进行包装，且记录日志，游标遍历接口是否需要使用迭代器模式封装。避免外部接口的对接逻辑过多侵入我们系统的核心逻辑，导致尾大不掉，无法快速响应接口改动。
- 代码逻辑是否符合规范，逻辑处理是否写到了相对应的层次。比如：总不能在controller里处理数据查询的逻辑，对吧。
# 可测试性
可测试性不仅意味着方便单元测试，还意味着可以快速验证猜想、修复bug；其他人可以通过编写单元测试，快速了解里面的逻辑。
- 单一职责。类、方法职责越单一，越简单，越容易写单元测试。
- 依赖注入，包括spring管理的组件、一些抽取逻辑的工具类。这样可以更容易的处理单元测试的环境问题。
- 避免使用ThreadLocal、全局变量等共享状态，尽量使用局部变量和显式传参。
- 执行无副作用。执行后不会修改数据等导致上下文变更，导致下次执行的结果与预期不符。
- 确定的输入，只会有唯一的输出。意味着单元测试的方法内不会有随机、当前时间等数据生成，结果也不依赖会变更的数据。
# 健壮性
- 校验参数、判断数据。
- 是否有潜在的栈溢出、死循环。如果使用递归，需要有确定的最多的递归层数；如果使用while(true)，应指定正常最多的循环次数；在触发这种疑似溢出、死循环时，应该告警到告警群，至少打一行error日志，且带上上下文数据。
- 自己造的轮子、包装的工具，比如：异步队列、延迟队列之类，是否有容量限制、容量监控告警。自己造的轮子，经验上很大概率上考虑不充分，有奇奇怪怪的bug。
- 边界值是否考虑充分，边界值复杂的逻辑是否有充分的单元测试。经验上，越多维度影响的逻辑，越容易因为边界值考虑不充分导致出问题。这种情况下，测试同学很难覆盖完全，开发同学写好各种情况下的单元测试，可以很大程度下避免。
- 是否考虑异常情况处理，特别是外部系统对接。
- 是否需要支持幂等；是否需要做对账、重试。
- 是否存在并发调用问题，需要加锁、唯一键。
# 一致性、可见性
- 发送消息前是否需要先提交事务。如果发送消息前不先提交事务，由于数据库隔离级别，有可能出现上下文数据查不出来。
- 是否需要强制指定PolarDB的会话一致性。
# 性能相关
- IO操作（包括远程调用、数据库查询、发送消息）次数是否过多，是否循环调用，是否可合并。
- 调用频率高、资源消耗大的业务是否需要，是否可通过缓存、流量合并的方式优化。
- 关键算法的时间复杂度。
- SQL连表个数。
- SQL索引是否生效。
- SQL查询行数是否有限制。
- SQL是否只查所需字段。
# 用户友好
- 对接外部系统返回的错误，不应直接返回给用户。而是我们内部转换成我们系统的错误提示，有必要则告警到告警群。最好带上相应的常见解决方法。
- 如果有输入、数据、业务异常，尽快响应给客户，并且有明确的错误提示，以及常见的解决方案。经验上，如果没做好这一点，会非常浪费技术值班同学的时间，而且用户会降低对系统的信心。
# 推荐的工具、资料
可以使用一些IDE的插件进行辅助Code Review，提高效率。它们都只能扫描其中某一些方面的内容，可以配合着使用。它们都有可能产生误，需要带着辩证的眼光使用。如果机器的性能不高，记得关闭实时扫描。
- Alibaba Java Coding Guidelines。该插件主要扫描规约包括：OOP规约、并发处理、控制语句、命名规约、常量定义、注释规范。
- CheckStyle。它只能检查编码格式和代码风格规范，如命名规范、Javadoc注释规范、空格规范、size度量（如过长的方法）、重复代码、多余Imports等。
- FindBugs。它能检查一些非常简单的bug，比如：引用类型相同判断错（使用了==判断引用类型，而不是equals()）、明显的空指针、未关闭资源。
- 阿里巴巴的《Java开发手册》。我们公司的代码规范就是以它为基础，以我们公司的实际情况出发，增删改了部分规约形成的。
# 最后几点
- 每次提交都是对系统功能、质量的提升。我们不可能单次提交就能把系统完善起来，代码也写得非常优雅。只要我们每次都是向着这些方向进步，就足够了。
- 避免破窗效应。一但引入质量差的代码，后续维护的人都会跟着这么写，长久下来，代码质量只会越来越差。
- 避免完美主义。由于时间、成本、开发者和审核者的认知等因素限制，不可能写出每个细节都完美的代码。
- 审核者注意态度。我们目的是为了团队、系统的提升，不是指责某个人。我们是同事、是工友，大家伙都是一样的。
- 看团队、项目情况进行Code Review，或不进行。团队人力资源紧张时，进行Code Reiew有些时候也不是好事。