1. 如何有效的做 CR?

很多同学都有这个疑问，如何结构化体系化的做 CR？如何综合应用各种手段尽快及早的发现代码问题和缺陷？

下面围绕这个实例，抛砖引玉，大家可以一起探讨；

1.1 CR 实例：8 行代码 21 问题

实例如下 ，短短 8 行代码，通过 CR 可以发现多少问题呢？21 处；这段代码谁写的不重要，探讨的重点是如何全面发现其中的问题和隐患；

1.2 CR 实例：如何结构化的 CR?

对这段代码，从背景了解，逻辑分析，异常分析，编码规范，非功能点，可测性来分析代码问题：

1.2.1. 背景了解

思考问题：代码块是要干什么？从上下文，方法签名和代码结构上快速看下代码：

Q1: 注释中，Line94，嗲吗出现音近拼写错误

Q2: 注释中，Line93（非 dev），Line94（生产），Line98（非 dev，非 test）不是很统一，应该是生产环境要保障是 https

Q3: 注释中，Line94，为啥生产中拿不到正确的 https, 原因是什么，什么情况触发，多大概率

生产环境可能拿不到正确的 schema, 是个异常的 CASE 处理

其中注释方法命名，出入参数没清晰表达场景

1.2.2. 逻辑分析

思考问题：代码究竟干了什么？选自己熟悉或则感兴趣的点开始：

• Q4: Line98，dbmode {String} dbmode： [dev test, stable, prod,...]为啥只考虑 dev 和 test?

• Q5: Line98，代码中是逆向判断，不符合条件（线上）会有很多，为啥不直接过滤 PROD， if(PROD) 比较好理解，简单；同时 case 数量也减少很多

• Q6: Line100，http 替换成 https，但是中间端口没有替换，是否存在此端口未开通 https 的情况代码做了什么清楚多了

1.2.3. 异常分析

思考问题：代码干不了什么？真实的环境复杂，数据多样，看看会那些情况：

• Q7: Line98，dev，test 都是小写，是否大写就不能匹配， 要是有枚举的可以统一处理

• Q8: Line98，dbMode 没做判空，如果 dbMode 为空，则会认为是生产环境

• Q9: Line99，httpGetRequest.getUri().xxx()可能报空指针异常

• Q10: Line99，uri.startsWith("http://") 需要考虑前面有空格的情况

• Q11: Line99，大写的 HTTP://是否也需要可也要匹配处理？现实中会出现这么多异常么，代码执行环境，输入最终是什么样的？

1.2.4. 编程规范分析

思考问题：代码怎么干最标准？各种情况逻辑情况有了，再看下能否最佳姿势表达：

• Q12: Line98，记得判断环境，有直接的函数的，不用自己写？ 之前主站有对这块治理过，是否可重用

• Q13: Line98, equals 方法比对对象写在左边

• Q14: Line98，if (!(StringUtil.equals(dbMode, "dev") || StringUtil.equals(dbMode, "test"))) if 中包含比较复杂判断，应该用变量替换例如:isSitEnv

• Q15: Line99, httpGetRequest.getUri()调用了三次，存储到变量中可以少调用两次

集团和蚂蚁都有编程规范，代码提交前可以自行扫描一下，部分代码门禁也有接入

代码中多处字面量（dev, test, http, https)，需要常量化，dbmode 适合枚举化

1.2.5. 非功能点分析

思考问题：代码有什么影响？跳出来再思考，代码虽小，也需要看下运行中链路，检查性能，稳定性等点：

• Q16: Line100, replaceFirst("^http", "https"); 使用正则，匹配比简单字符操作要耗时

• Q17: Line100, replaceFirst 中，每次都会 new*complie Pattern，大量命中存在不必要的重复 compile pattern 时间

• Q18: Line100, 从 comment 中看出这个一个异常情况，线上如果触发了，需要一种方式能感知，例如增加打印日志并需要配置监控，目前没有日志无法感知，也无法调查到 root cause 发现功能之外情况，同时需要考虑三板斧

1.2.6. 可测试行分析

思考问题：代码怎么覆盖又快又好？

• Q19: Line100, 由于限定了环境，在线下环境没发功能测试进行覆盖，最好单测覆盖最简单

• Q20: Line100, 线上触发时也无日志打印，不具备可观察性；

• Q21: Line100, 如果不加日志，在验收或则开量时进行 double check 出入参的正确性（或验收用例）

2. 抽象 1 步，如何复制和复用？

这里共 21 个问题，其实每个问题都能对应到快速 check list 中

可以看出：异常情形，编程规范，功能逻辑和注释这块问题比较多，非功能性和可测性问题也存在

2.1 问题分类

2.1.1 需求和上下文

每个 Reviewer 都会关注的地方，好的注释，代码结构很容易人理解：

• Q3: Line94，为啥生产中拿不到正确的 https, 原因是什么，什么情况，多大概率

• 这个是需求，是提前，为啥要做这个事情。在 CR 时需要需要了解，特别是对口的开发测试同学，否则发现不了基本的问题

2.1.2 代码逻辑

不管是否熟悉业务，都能发现逻辑问题：

• Q8: Line98，dbMode 没做判空，如果 dbMode 为空，则会认为是生产环境

• 这个是异常数据，是否真的存在，存在了有什么影响

2.1.3 代码规范

这种问题可能太多了，经验和方法也有很多。Follow 最佳实践和惯例，可以提高可读性和避免采坑:

• Q14: Line98，if (!(StringUtil.equals(dbMode, "dev") || StringUtil.equals(dbMode, "test"))) if 中包含比较复杂判断，应该用变量替换例如:isSitEnv

• 这个是编程规范问题，字面值常量化（枚举化），复杂判断，代码可读性,

2.1.4 业务逻辑

业务背景知识对实现的完整性至关重要:

• Q4: Line98，dbmode {String} dbmode： [dev, sit, stable, gray, prod,...]为啥只考虑 dev 和 test?

• 这个是常见错误，忽视了客观数据；很多异常 case 都是由此产生的

2.1.5 设计合理

合理的设计，要结合上线文来看，会有对比和取舍：

• Q17: Line100, replaceFirst 中，每次都会 new*complie Pattern，大量命中存在不必要的重复 compile pattern 时间

• 这个是设计问题（性能考量），string 的 replaceFirst 调用正则，一次 compile 没问题，也逻辑很清楚

• 如果大量请求，正则重复 compile，则有不必要重复耗时计算，这里主要识别 replaceFirst 的实现机制，根据上下文来选择

2.1.6 非功能

需要跳出功能点来看，从静态的代码，看到质量的更多面：

• Q18: Line100, 从 comment 中看出这个一个异常情况，线上如果触发了，需要一种方式能感知，例如增加打印日志并需要配置监控，目前没有日志无法感知，也无法调查到 root cause

• 大家能看到有很多防御性代码，避免了线上的报错，是大家普遍接受的。过多防御代码，让系统逐步减少对异常感知能力，从而隐藏了真正的问题。

• 另外防御性代码对今后的维护性造成了隐患，不清楚是否能改造。

2.1.7 可测性

可测性是测试同学，也是开发需要考虑的点：

• Q19: Line100, 由于限定了环境，在线下环境没发功能测试进行覆盖，最好单测覆盖最简单

• 这个是可测性的问题，测试同学在 CR 时需要考虑这块，考虑通过什么手段来覆盖；很多测试不一定要从集成后进行功能测试；CR 和单测是很好的方法。

2.2 结构化 CR

上述发现的问题和我们 CR 的方式很有关系，每个人进入 CR 的时机不一样，关注点也不一样。下面是从 CR 的简单流程，每个人 CR 时都能从中找一个切入点，根据经验和已有规则，能提出自己问题。

2.3 CR 常见规则

下图是一个 Expert Code Reviewer 的想法，正常大家在 Code Review 可能不会考虑这么多和全面，哪些可以帮助快速有效 CR 发现问题。根据之前出现的问题，下面列出最常见 check 点，帮助大家有效 CR。

2.3.1 可读性

• 评论是否清晰有用？变量、类、方法的命名是否明确，可辨识

• 代码可读性，将来其他同学能轻松理解并使用此代码

• 阿里巴巴集团 JAVA 代码规范-基础规约可读性部分

这些规范可由门禁代码扫描[静态扫描规则库]发现部分，可适当关注。

• 注释规约

• 常量定义

• 格式规约

• 命名规约

2.3.2 代码逻辑

• 代码是否实现 DEV 的意图

• 边界问题，如数组越界，获取 object key，业务逻辑的边界（没有的情况、全等、全 null、全 undefined 等）

• 资源泄露（内存泄露，文件句柄未释放，网络端口占用等）

• 算术计算溢出（赋值截断，精度提升，移位，类型转化等）

• 线程 &并发处理（ThreadLocal 退出清理，一锁二判三更新四释放等）

• 异常数据处理(数据约定，长度，类型，默认值等）

• 工具使用姿势是否正确（了解工具背后逻辑)

• 工具常见问题，如 map.put key=null 报错，switch(param=null)报错

2.3.2 代码规范

• 阿里巴巴集团 JAVA 代码规范-基础规约

• 控制语句

• OOP 规约

• 集合处理

• 并发处理

• 其他平台语言开发参考：

• 集团 iOS 开发规约

• 集团 Android 开发规约

• 集团前端开发规约

• 集团 C++规约

是否 follow 业界 Best Practice Oracel Java Language Best Practices

2.3.3 业务逻辑

• 功能实现是否完整性

• 是否遗漏关联场景

• 异常情形处理

• 对依赖方实现，架构和规划有了解

2.3.4 设计合理性

• 明确真实的业务需求和场景，真实环境中怎么使用的

• 是否基于未确定的假设

• 是否重复造轮子，有没有漏洞和留坑，有限制

• 是否和架构，系分匹配，能否满足业务需求

• 是否 follow 常见设计规约，如阿里巴巴设计规约, 蚂蚁金服 DB 设计规范

• 异常日志

• MySQL 规约

• OceanBase

• 蚂蚁中间件

• 蚂蚁二方库

• 蚂蚁安全

• 服务器规约

• 国际化设计 follow 国际际化规约，其实个人认为所有应用默认就应 follow

• 错误码设计

• 常见性能（是否有更优操作方法，是否有重复耗时任务， 在大量请求下是否触发性能问题）

• 防错设计
  

2.3.5 非功能

常见的点，但不限于

1. 稳定性，上下游链路稳定性

• 兼容性（代码兼容性， 数据兼容性，协议兼容性）

• 系统依赖（高等级稳定依赖低等级系统）

• 数据库和缓存（SQL 性能，是否能走到索引，缓存多入口刷新，失败补偿，缓存 &持久化不一致）

• 流量和性能（服务重试+长耗时，下游服务负载过重，自身限流）

• 资金安全（前端不进行金额计算，幂等字段，并发流水更新）

• 服务隔离（是否一个失败是否会触发整体失败）

2. 安全性

• 垂直权限和水平权限

• 数据脱敏和防泄漏（不限于日志，前端返回）

• 用户请求有效校验

• 客户端安全请参考：

• 蚂蚁金服 h5 安全开发规范

• 蚂蚁金服 android 应用安全开发规范

• 蚂蚁金服 IOS 应用安全开发规范

3. 可诊断性

• 关键路径是否有日志

• 日志信息是否可排查，能否各端串联起来，如：线程加上名称等，异常时打印信息是否可用于复原场景

4. 可监控，可灰度，可应急

• 关键业务有日志埋点用于监控，并发现问题

• 该功能可灰度方式和策略

• 数据变更是否可灰度，怎么灰度的

• 是否需要和有应急能力

5. 可测性

• 是否可测, 是否满足可观察性，可操作性，可自动化等

• 什么方式覆盖和难点，CR 覆盖，单测覆盖，功能，还是专项，是否需要额外可测性改造

• 单元测试规约，很多点对功能自动化也使用

3 再抽象 1 步，如何系统性预防和发现暴露出的问题？

3.1 问题背后的问题

为什么会有这么问题呢？问题多属于注释，异常情形，编程规范，功能逻辑；下面具体的原因分析：

这里有人的经验，有方法，复杂环境和基础设施原因，都可以落到这个九宫格中。

究竟怎么防范和发现问题呢？人员经验可以积累，方法可以沉淀和传授和基础设施（技术，数据，AI)可以逐步建设，下面说下自己的想法：

3.2 人员

• 提升质量意识，全面认识质量；

• 如代码复杂度高，可读性，测试成本也高；发现治理根因比问题防范更重要等

• 主动通过持续技术运营，技术分享等来弥补技术不足和积累经验

• 在持续 Code Review 中学习，类似的问题就会越少

• 防错设计意识

建议参考The Clean Coder: A Code of Conduct for Professional Programmers (Robert C. Martin Series)

3.3 方法

• 养成良好 CR 方式，提交 CR 时 double check 一次加强自检，对 Author 和 Reviewer 来说同样重要：

• 具体可参考Google的CR最佳实践（英文版）可以组织 CR 学习

• 设计方法

• 不少假设和前提需要确定，同时需要数据和工具来支持，在正确前提下做设计，防范

• 设计和实现方式，很多问题都有共性，或有存在的解，开发者和 Reviewer 都需要持续积累

• 参考标准集团开发规约和业界最佳实践

• 持续质量运营

• 持续补充代码规范和 Best Practice 到扫描工具中并分享

• 经典或常见问题了解，如国际 daily quiz 和 daily bug

3.4 基础设施（技术解法）