作者：方基成(润甫)
作为杰出工程文化的一部分，Code Review 其实一直在进行中，仅仅各团队根据自身状况张驰有度，松紧或许也不一，这里简略梳理一下 CR 的办法和团队实践。
为什么要CR
-
提早发现缺陷
在 CodeReview 阶段发现的逻辑错误、事务了解误差、功用危险等时有发生， CR 能够提早发现问题。 -
提高代码质量
首要体现在代码健壮性、规划合理性、代码高雅性等方面，持续 CodeReview 能够提升团队全体代码质量。 -
一致标准和风格
集团编码标准自不必说，关于代码风格要不要一致，或许会有不同的看法，个人观点关于风格也不强求。但代码其实不是写给自己看的，是写给下一任看的，就像经常被戏弄的“程序员不喜欢写注释，更不喜欢他人不写注释”，代码风格的一致更有助于代码的可读性及继任者的快速上手。 -
防止架构腐烂
架构的维护者是谁？仅靠架构师或应用 Owner 是远远不够的，需求一切成员的努力，所谓人人都是架构师。架构防腐最好前置在规划阶段，但 CodeReview 作为对终究产出代码的查看，也算是最终一道关键工序。 -
常识分享
每一次 CodeReview，都是一次常识的分享，磨合必定时刻后，团队成员间会你中有我、我中有你，集百家之所长，融百家之所思。一同，事务逻辑都在代码中，团队 CodeReview 也是一种新人事务细节学习的途径。 -
团队一致
经过多次讨论与沟通，逐步达成团队一致，特别是对架构了解和规划准则的认知，在一致的根底上团队也会更有凝聚力，特别是在较多新人加入时尤为重要。
参考之资
2.1 某大厂A
非常重视 Code Review，根本上代码需求至少有两位以上 Reviewer 审阅通往后，才会让你 Check In。
2.1.1 代码评定准则
-
假如改变到达能够提升体系全体代码质量的程度，就能够让它们经过，即使它们或许还不完美。这是一切代码评定准则的最高准则。
-
世界上没有“完美”的代码，只需更好的代码。评定者不应该要求代码提交者在每个细节都写得很完美。评定者应该做好修正时刻与修正重要性之间的权衡。
2.1.2 代码评定准则
-
以客观的技能要素与数据为准，而非个人偏好。
-
在代码款式上，遵从代码款式攻略，一切代码都应与其保持一致，任何与代码款式攻略不一致的观点都是个人偏好。但假如某项代码款式在攻略中未提及，那就承受作者的款式。
-
任务涉及软件规划的问题，都应取决于根本规划准则，而不应由个人喜好来决议。当一同有多种可行计划时，假如作者能证明（以数据或公认的软件工程原理为根据）这些计划根本差不多，那就承受作者的选项；不然，应由标准的软件规划准则为准。
-
假如没有可用的规矩，那么审阅者应该让作者与当时代码库保持一致，至少不会恶化代码体系的质量。（一旦恶化代码质量，就会带来破窗效应，导致体系的代码质量逐渐下降）
2.1.3 代码审阅者应该看什么
-
规划：代码是否规划杰出？这种规划是否适合当时体系？
-
功用：代码完结的行为与作者的期望是否相符？代码完结的交互界面是否对用户友爱？
-
杂乱性：代码能够更简略吗？假如将来有其他开发者运用这段代码，他能很快了解吗？
-
测验：这段代码是否有正确的、规划杰出的自动化测验？
-
命名：在为变量、类名、办法等命名时，开发者运用的名称是否明晰易懂？
-
注释：一切的注释是否都一望而知？
-
代码款式：一切的代码是否都遵从代码款式？
-
文档：开发者是否一同更新了相关文档？
2.2 某大厂B
-
在开发流程上专门有这个环节，排期会明晰排进日程，比方5天开发会排2天来做代码审阅，分为代码自审、交叉审阅、会集审阅。
-
有明晰的量化目标，如8人时审阅/每千行代码，8个以上非提示性有用问题/每千行代码。
2.3 某大厂C
-
推行 Code Owner 机制，每个代码改变必须有 Code Owner 审阅经过才能够提交。
-
一切的一线工程师，不管职级凹凸，最重要的工程输出准则是“show me the code”，而 Code Review 是最能够反应这个客观输出的。
-
尽量让每个人的 Code Review 参加状况都公开透明，每个改变发送给项目合作者，及转发到小组内成员，小组内任何人都能够去 Review 其他人的代码。
-
明晰每个人的考评和 Code Review 表现相关，包含 Code Review 输出状况及提交代码的质量等。
咱们怎么做 CR
3.1 作为代码提交者
- 主张机遇：主张 Code Review 尽量提早，开发过程小步快跑
-
代码行数：提交 Code Review 的代码行数最好在400行以下。根据数据分析发现，从代码行数来看，超越400行的 CR，缺陷发现率会急剧下降；从 CR 速度来看，超越500行/小时后，Review 质量也会大大降低，一个高质量的 CR 最好控制在一个小时以内。
-
明晰目的：编写语义明晰的标题（必填）和描绘（选填，能够包含背景、思路、改造点和影响面、风险等）
-
善用工具：IDEA 打开编码规约实时检测，削减代码款式、编码规约等根底性问题
阿里编码规约插件： github.com/alibaba/p3c…
3.2 作为代码评定者
3.2.1 评定规模
首要从两方面来评定：
-
代码逻辑
-
-
功用完整：代码完结是否满意功用需求，完结上有没有需求的了解误差，对用户是否友爱；
-
逻辑规划：是否考虑了大局规划和兼容现有事务细节，是否考虑边界条件和并发控制；
-
安全危险：是否存在数据安全危险及灵敏信息走漏，如越权、SQL注入、CSRF、灵敏信息未脱敏等；
-
功用危险：是否存在损害功用的危险，如死锁、死循环、FullGC、慢SQL、缓存数据热门等；
-
测验用例：单元测验用例的验证逻辑是否有用，测验用例的代码行覆盖率和分支覆盖率；
-
-
代码质量
-
-
编码标准：命名、注释、范畴术语、架构分层、日志打印、代码款式等是否契合标准
-
可读性：是否逻辑明晰、易了解，防止运用奇淫巧技，防止过度拆分
-
简洁性：是否有重复可简化的杂乱逻辑，代码杂乱度是否过高，契合KISS和DRY准则
-
可维护性：在可读性和简洁性根底上，是否分层明晰、模块化合理、高内聚低耦合、遵从根本规划准则
-
可扩展性：是否仅仅是满意一次性需求的代码，是否有必要的前瞻性扩展规划
-
可测验性：代码是否便利写单元测验及分支覆盖，是否便于自动化测验
-
3.2.2 评定注意事项
-
赶快完结评定
-
防止过度寻求完美
-
明晰谈论是否要处理
-
防止运用反问句来评价
咱们首要是经过交叉 CR、会集 CR 相结合的方式，由应用 Owner+SM+架构师+TL完结。
CR 怎么防止流于方式
CR 流于方式的要素很多，大约如下：
-
不认同 CodeReview
-
-
评定者的姿态？有没有带来优点？有没有从中收成？这些都会直观影响团队成员的认可度
-
每个 Review 主张的提出都是一次思维沟通，谈论要友爱、中肯、详细，防止教条式及负面词汇，在遵守评定准则下，一同尊重特性展示
-
团队会集 CodeReview 尽量不要太正式和严厉，轻松的气氛下更有助于互相了解，来点水果，聊聊事务聊聊代码
-
在 Review 过程有时候会陷入谁对谁错的争论，只需是为了寻求真理辩证的去看问题，哪怕是讨论再激烈也是有收成的，注意只对事不对人。
-
-
CodeReview 后改动太大
-
-
发布前发现问题多，改动太大，影响项目计划
-
大项目要求编码前规划评定，小需求能够事先Review规划思路，防止最终的惊喜
-
每次 Review 的代码行数最好控制在数百行以内
-
-
评定者没有足够时刻
-
-
评定者在任务安排上尽量预留好时刻
-
赶快评定，代码在百行以内及时响应，在千行以内当日完结
-
-
评定者不了解事务和代码
-
-
代码提交人编写明晰的标题和描绘
-
有必要的状况下评定者需求了解PRD
-
评定者需求提早了解体系和代码
-
-
Review 主张未修正
-
-
这一点极为重要，需求对修正后的代码再次 Review，确保了解一致，以及预防带问题上线
-
应用能够设置 Review 主张需悉数处理的卡点，一同关于非必需修正的主张能够进行打标或说明
-
CR 实践中发现的几个常见代码问题
笔者对个人 CR 谈论问题做了个大约计算，Bug 发现数占比约4%（直接或潜在Bug），重复代码数占比约5%，其他还有标准、安全、功用、规划等问题。在CR代码质量时，能够参考《重构：改进既有代码的规划》，书中所列的22种坏味道在CR中根本都会遇到。而此处咱们首要聚集以下几个常见问题：
5.1 DRY
DRY 是 Don’t Repeat Yourself 的缩写，DRY 是 Andy Hunt 和 Dave Thomas’s 在《 The Pragmatic Programmer 》一书中提出的中心准则。DRY 准则描绘的重复是常识和目的的重复，包含代码重复、文档重复、数据重复、表征重复，咱们这里重点讲讲代码重复。
5.1.1 代码重复
《重构》中对“Duplicated Code（重复代码）”的描绘：坏味道行列中首战之地的便是Duplicated Code。假如你在一个以上的地址看到相同的程序结构，那么能够必定：设法将它们合而为一，程序会变得更好。
最单纯的Duplicated Code便是“同一个类的两个函数含有相同的表达式”。这时候你需求做的便是采用Extract Method (110)提炼出重复的代码，然后让这两个地址都调用被提炼出来的那一段代码。
另一种常见状况便是“两个互为兄弟的子类内含相同表达式”。要防止这种状况，只需对两个类都运用Extract Method (110)，然后再对被提炼出来的代码运用Pull Up Method (332)，将它推入超类内。假如代码之间仅仅类似，并非彻底相同，那么就得运用Extract Method (110)将相似部分和差异部分割开，构成独自一个函数。然后你或许发现能够运用Form Template Method (345)获得一个Template Method规划方式。假如有些函数以不同的算法做相同的事，你能够选择其间较明晰的一个，并运用Substitute Algorithm (139)将其他函数的算法替换掉。
假如两个毫不相关的类呈现Duplicated Code，你应该考虑对其间一个运用Extract Class (149)，将重复代码提炼到一个独立类中，然后在另一个类内运用这个新类。可是，重复代码所在的函数也或许的确只应该归于某个类，另一个类只能调用它，抑或这个函数或许归于第三个类，而另两个类应该引证这第三个类。你必须决议这个函数放在哪儿最合适，并确保它被安顿后就不会再在其他任何地方呈现。
代码重复的几种场景：
-
一个类中重复代码笼统为一个办法
-
两个子类间重复代码笼统到父类
-
两个不相关类间重复代码笼统到第三个类
CASE：
反例
private BillVO convertBillDTO2BillVO(BillDTO billDTO) {
if (billDTO == null) {
return null;
}
BillVO billVO = new BillVO();
Money cost = billDTO.getCost();
if (cost != null && cost.getAmount() != null) {
billVO.setCostDisplayText(String.format("%s %s", cost.getCurrency(), cost.getAmount()));
}
Money sale = billDTO.getSale();
if (sale != null && sale.getAmount() != null) {
billVO.setSaleDisplayText(String.format("%s %s", sale.getCurrency(), sale.getAmount()));
}
Money grossProfit = billDTO.getGrossProfit();
if (grossProfit != null && grossProfit.getAmount() != null) {
billVO.setGrossProfitDisplayText(String.format("%s %s", grossProfit.getCurrency(), grossProfit.getAmount()));
}
return billVO;
}
正例
private static final String MONEY_DISPLAY_TEXT_PATTERN = "%s %s";
private BillVO convertBillDTO2BillVO(BillDTO billDTO) {
if (billDTO == null) {
return null;
}
BillVO billVO = new BillVO();
billVO.setCostDisplayText(buildMoneyDisplayText(billDTO.getCost()));
billVO.setSaleDisplayText(buildMoneyDisplayText(billDTO.getSale()));
billVO.setGrossProfitDisplayText(buildMoneyDisplayText(billDTO.getGrossProfit()));
return billVO;
}
private String buildMoneyDisplayText(Money money) {
if (money == null || money.getAmount() == null) {
return StringUtils.EMPTY;
}
return String.format(MONEY_DISPLAY_TEXT_PATTERN, money.getCurrency(), money.getAmount().toPlainString());
}
5.1.2 DYR 实践忠告：
-
不要借用 DRY 之名，过度提早笼统，请遵从 Rule of three 准则。
-
不要过度寻求 DRY，破坏了内聚性，实践中需求平衡复用与内聚。
5.2 Primitive Obsession
《重构》中对“Primitive Obsession（根本类型偏执）”的描绘：大多数编程环境都有两种数据：结构类型允许你将数据组织成有意义的方式；根本类型则是构成结构类型的积木块。结构总是会带来必定的额定开支。它们或许代表着数据库中的表，假如只为做一两件事而创立结构类型也或许显得太费事。
目标的一个极大的价值在于：它们模糊（乃至打破）了横亘于根本数据和体积较大的类之间的边界。你能够轻松编写出一些与言语内置（根本）类型无异的小型类。例如，Java 就以根本类型表明数值，而以类表明字符串和日期——这两个类型在其他许多编程环境中都以根本类型表现。
目标技能的新手一般不愿意在小任务上运用小目标——像是结合数值和币种的 money 类、由一个起始值和一个完毕值组成的 range 类、电话号码或邮政编码（ZIP)等的特殊字符串。你能够运用 Replace Data Valuewith Object (175)将原本独自存在的数据值替换为目标，然后走出传统的洞窟，进入炙手可热的目标世界。假如想要替换的数据值是类型码，而它并不影响行为，则能够运用 Replace Type Code with Class (218)将它换掉。假如你有与类型码相关的条件表达式，可运用Replace Type Codewith Subclass (213)或Replace Type Code with State/Strategy (227)加以处理。
假如你有一组应该总是被放在一同的字段，可运用 Extract Class(149)。假如你在参数列中看到根本型数据，无妨试试 IntroduceParameter Object (295)。假如你发现自己正从数组中选择数据，可运用 Replace Array with Object (186)。
给咱们的启示首要有两点：
-
大部分事务场景和言语环境下，结构化类型导致的开支根本能够忽略
-
结构化类型带来更明晰的语义和复用
CASE：
反例
@Data
public class XxxConfigDTO implements Serializable {
private static final long serialVersionUID = 8018480763009740953L;
/**
* 租户ID
*/
private Long tenantId;
/**
* 工商税务企业类型
*/
private String companyType;
/**
* 企业名称
*/
private String companyName;
/**
* 企业纳税人识别号
*/
private String companyTaxNo;
/**
* 审单职工工号
*/
private String auditEmpNo;
/**
* 审单职工名字
*/
private String auditEmpName;
/**
* 跟单职工工号
*/
private String trackEmpNo;
/**
* 跟单职工名字
*/
private String trackEmpName;
}
正例
@Data
public class XxxConfigDTO2 implements Serializable {
private static final long serialVersionUID = 8018480763009740953L;
/**
* 租户ID
*/
private Long tenantId;
/**
* 企业信息
*/
private Company company;
/**
* 审单职工信息
*/
private Employee auditEmployee;
/**
* 跟单职工信息
*/
private Employee trackEmployee;
}
@Data
public class Company {
/**
* 工商税务企业类型
*/
private String companyType;
/**
* 企业名称
*/
private String companyName;
/**
* 企业纳税人识别号
*/
private String companyTaxNo;
}
@Data
public class Employee {
/**
* 职工工号
*/
private String empNo;
/**
* 职工名字
*/
private String empName;
}
其实便是怎么去笼统，关于特定范畴的目标能够参考 DDD 里边的 Domain Primitive（DP）。
5.3 分布式锁
5.3.1 未处理锁失败
private void process(String orderId) {
// do validate
try {
boolean lockSuccess = lockService.tryLock(LockBizType.ORDER, orderId);
if (!lockSuccess) {
// TODO 此处需求处理锁失败，重试或抛出反常
return;
}
// do something
} finally {
lockService.unlock(LockBizType.ORDER, orderId);
}
}
分布式锁的目的是为了防止并发冲突和确保数据一致性，锁失败时未处理直接返回，会带来非预期成果的影响，除非明晰失败可放弃。
5.3.2 手写解锁简单遗失
上面的加锁和解锁都是手动编写，而这两个动作一般是成对呈现的，在手动编写时简单发生遗失解锁而导致线上问题，推荐封装一个加解锁的办法来完结，会愈加安全和便利。
private void procoess(String orderId) {
// do validate
Boolean processSuccess = lockService.executeWithLock(LockBizType.ORDER, orderId, () -> doProcess(orderId));
// do something
}
private Boolean doProcess(String orderId) {
// do something
return Boolean.TRUE;
}
// LockService
public <T> T executeWithLock(LockBizType bizType, String bizId, Supplier<T> supplier) {
return executeWithLock(bizType, bizId, 60, 3, supplier);
}
public <T> T execteWithLock(LockBizType bizType, String bizId, int expireSeconds, int retryTimes, Supplier<T> supplier) {
// 尝试加锁
int lockTimes = 1;
boolean lock = tryLock(bizType, bizId, expireSeconds);
while(lockTimes < retryTimes && !lock) {
try {
Thread.sleep(10);
} catch (Exception e) {
// do something
}
lock = tryLock(bizType, bizId, expireSeconds);
lockTimes++;
}
// 锁失败抛反常
if (!lock) {
throw new LockException("try lock fail");
}
// 解锁
try {
return supplier.get();
} finally {
unlock(bizType, bizId);
}
}
5.3.3 加锁 KEY 无效
private void process(String orderId) {
// do validate
try {
// 此处加锁类型与加锁KEY不匹配
boolean lockSuccess = lockService.tryLock(LockBizType.PRODUCT, orderId);
if (!lockSuccess) {
// TODO 重试或抛出反常
return;
}
// do something
} finally {
lockService.unlock(LockBizType.PRODUCT, orderId);
}
}
注意加锁类型与加锁 KEY 在同一个维度，不然加锁会失效。
5.4 分页查询
5.4.1 彻底没有分页
反例
private List<OrderDTO> queryOrderList(Long customerId) {
if (customerId == null) {
return Lists.newArrayList();
}
List<OrderDO> orderDOList = orderMapper.list(customerId);
return orderConverter.doList2dtoList(orderDOList);
}
正例
private Page<OrderDTO> queryOrderList(OrderPageQuery query) {
Preconditions.checkNotNull(query, "查询条件不能为空");
Preconditions.checkArgument(query.getPageSize() <= MAX_PAGE_SIZE, "分页size不能大于" + MAX_PAGE_SIZE);
// 分页size一般由前端传入
// query.setPageSize(20);
long cnt = orderMapper.count(query);
if (cnt == 0) {
return PageQueryUtil.buildPageData(query, null, cnt);
}
List<OrderDO> orderDOList = orderMapper.list(query);
List<OrderDTO> orderDTOList = orderConverter.doList2dtoList(orderDOList);
return PageQueryUtil.buildPageData(query, orderDTOList, cnt);
}
没有分页的列表查询对 DB 功用影响非常大，特别是在项目初期，因为数据量非常小问题不明显，而导致没有及时发现，会给未来留坑。
5.4.2 分页 size 太大
反例
private Page<OrderDTO> queryOrderList2(OrderPageQuery query) {
Preconditions.checkNotNull(query, "查询条件不能为空");
query.setPageSize(10000);
long cnt = orderMapper.count(query);
if (cnt == 0) {
return PageQueryUtil.buildPageData(query, null, cnt);
}
List<OrderDO> orderDOList = orderMapper.list(query);
List<OrderDTO> orderDTOList = orderConverter.doList2dtoList(orderDOList);
return PageQueryUtil.buildPageData(query, orderDTOList, cnt);
}
分页 size 的巨细并没有一个固定的标准，取决于事务需求、数据量及数据库等，但动辄几千上万的分页 size，会带来功用瓶颈，而大量的慢 SQL 不但影响客户体会，对体系稳定性也是极大的危险。
5.4.3 超多分页慢 SQL
反例
<!-- 分页查询订单列表 -->
<select id="list" parameterType="com.xxx.OrderPageQuery" resultType="com.xxx.OrderDO">
SELECT
<include refid="all_columns"/>
FROM t_order
<include refid="listConditions"/>
ORDER BY id DESC
LIMIT #{offset},#{pageSize}
</select>
正例
<!-- 分页查询订单列表 -->
<select id="list" parameterType="com.xxx.OrderPageQuery" resultType="com.xxx.OrderDO">
SELECT
<include refid="all_columns"/>
FROM t_order a
INNER JOIN (
SELECT id AS bid
FROM t_order
<include refid="listConditions"/>
ORDER BY id DESC
LIMIT #{offset},#{pageSize}
) b ON a.id = b.bid
</select>
以上 bad case 的 SQL 在超多页分页查询时功用极其低下，存在多次回表乃至 Using Filesort 的问题，在阿里巴巴编码标准中也有明晰的躲避计划，此处不打开。
最终，咱们工程师的智慧结晶都尽在代码之中，而 Code Review 能够促进结晶愈加清莹通透、纯真无瑕、精美完美，值得我们一同持续精进！
