CR（Code Review）目的

提高代码质量和可维护性、可读性等
通过交叉排查缺陷，查漏补缺, 发现一些潜在的问题点等
最佳实践, 能够更好更快的完成任务的方法
建立团队意识，知识分享, Review他人代码时, 其实也是一个学习的过程, 自己也会反思&总结，团队成员在相互督促与改进中共同成长

本文涉及名词

cr: code review
cl: change list，指这次改动
reviewer: cr中的那个review的人
author：作者，也就是本次CL的开发者，author称为开发者，也即是“CL的作者”。

Code Review前提

基于CR是在CL完成后进行的环节，是起到查漏补缺的作用，所以要尽量做到有规范标准，也可以让团队更快更好的接受和实施，所以要有以下前提：

团队有约定的规范，并尽量约束成员按照规范编码：
- 编码规范, 如变量命名, 文件命名规范等
- 核心代码逻辑清晰且有必要注释说明
- 设计原则, 如单一职责原则, 最少知识原则等
- 分支管理策略
单次提交Review的代码量尽量完整独立，即开发者完成初步结构设计，或者完成一个相对完整的小模块达到可提交阶段
将别人提交的代码视作自己将维护的代码，个人标准与集体标准对齐
不能形式化、对事不对人，对改进意见未做修改的提交不能上线

Code Review标准及技巧

注意以下技巧及标准：

不纠结编码风格。编码风格交给eslint（IDE安装eslint并启用）
代码性能。大数据处理、重复遍历渲染等
代码注释。字段注释、文档注释等，注意使用ts，避免使用any，核心代码逻辑清晰且有必要注释说明
代码结构格式化清晰
变量、方法等命名符合规范，不要无异议的命令
代码可读性。过多嵌套、低效冗余代码、功能独立、可读性变量方法命名等，无异议的命名对代码可读性带来负帮助
代码可扩展性。功能方法设计是否合理、模块拆分等
单次提交Review的代码量尽量完整独立
粒度要尽可能小，一个组件一个方法均可
覆盖度比深度重要，覆盖度追求100%
单次查看代码不多于500行, 人的精力有限, 一次审查太多的代码, 收益可能不理想
单次审查建议不要超过30分钟，reviewer 尽量由项目责任人组成，关注代码逻辑，无需逐字逐句理解。

Code Review流程

8a6066f1c4a94b2aba3e3590c3a6e4d4_tplv-k3u1fbpfcp-watermark.png 同时分为两种场景：

1. 需求开发：

开发者根据 Develop 分支建立个人开发分支(英文全名，如 feature-xxx-zl)，每天自行 Merge Develop 分支代码；根据开发情况，定期向 Develop 分支发起 Merge Requests 请求，然后通知相关人进行 codereview，根据代码情况查看是否进行合并，合并与否都需要留下coment

2. Bug修改

开发者根据 Master 分支建立临时 hotfix 分支，根据开发情况，开发者根据 hotfix 分支建立个人开发分支(英文全名，如 hotfix-xxx-zl)，每天自行 Merge hotfix 分支代码；然后通知相关人进行 codereview，根据代码情况查看是否进行合并，合并与否都需要留下 coment，合并后相关开发者删除临时 hotfix 分支。

同时要注意以下规范

git提交规范

按照团队git commit提交规范执行，如果提交代码时：

不标注信息
不及时commit
提交时标注的信息不规范等都会严重阻扰review，除了常规的描述信息外,还可以按类型等备注:
feat: 新特性
fix: 修改问题
refactor: 代码重构
docs: 文档修改
chore: 其他修改
test: 测试用例修改
style: 代码格式修改等等

规范展开细讲

基于我们的标准可以稍微展开细讲以下，可关注以下几方面：

编码风格：

1. 代码可读性

即Code Review时能否非常容易的理解代码逻辑, 如果不能, 那意味着代码的可读性要进行改进.

2. 命名

命名对可读性非常重要, 我个人倾向于函数名/类名长一点都没关系, 但必须能清晰的表明函数/类的作用.
英语用词尽量准确, 哪怕需要借助翻译工具, 也是值得的. 但有一点需要注意, 如果使用的单词很冷门, 都没人认识, 那就不要使用.

3. 函数体长度/类长度

函数体太长, 不好阅读, 一般建议不要超过50行
类太长, 如超过10000行, 那可能就要看下是否违反了"单一职责"原则.

4. 参数个数

不要太多, 一般不要超过5个, 超过5个, 建议使用对象

5. 注释

恰到好处的注释, 能够帮助我们理解函数/类的作用.

代码架构/设计方面应该注意：

1. 单一职责原则

这是经常被违背的原则, 也是最难运用好的原则.

一个类只做一类相关的事情
一个函数/方法, 最好只做一件事情

2. 行为是否统一

1) 什么是行为统一? 例如:

错误处理是否统一
错误提示是否统一
弹出框是否统一
...

2) 同一逻辑/行为, 有没有执行同样的代码路径

低质量的代码一个特征是, 同一逻辑/行为, 在不同的地方或不同的方式触发时, 没有执行同样的代码路径(产生出不同的结果), 或者是各处copy一份实现, 导致非常难以维护.

3. 代码污染

代码有没有对其他模块强耦合

4. 重复代码

主要看有没有把公用组件, 可复用的代码、函数抽取出来

5. 开放-封闭原则

简单理解是, 看代码好不好扩展

6. 健壮性

核心数据有没有强制校验?
对业务有没有考虑完整, 逻辑是否健壮
有没有潜在的bug?
有没有内存泄露?有没有循环依赖?

7. 错误处理

有没有很好的Error Handling? 如网络出错, IO出错等.

8. 面向接口编程/面向对象接口编程

主要看有没有进行合适的抽象, 把一些行为抽象为接口

9. 效率/性能

客户端程序对频繁的消息和较大数据等耗时操作是否处理得当
关键算法的时间复杂度是多少? 有没有潜在的性能瓶颈?

10. 代码重构

新的改动是打补丁, 让代码质量继续恶化, 还是对代码质量提升有帮助?

低质量代码示例

1. 低质量代码常见以下特征

违反"单一职责"原则
同一逻辑/行为, 通过不同的方式触发时, 不会执行同样的代码路径
缺少注释
函数/类命名乱, 词不达意, 或"挂羊头卖狗肉" 如下示例：

export function setDefaultImg(res) {
    let imgUrl = getImagePrefixUrl()
    let previewImgSrc = require('@/asserts/images/icon.png')
    if(res.imgId) {
        previewImgSrc = `${imgUrl}${res.imgId}.${res.type}?t=${+new Date()}`
    }
    return previewImgSrc
}

上例函数, 其实是想对传入的参数进行判断, 如果值无效, 取默认图片, 否则返回拼接地址好的img src相对地址. 从函数的实现来看, 有如下几个问题:

函数名setDefaultImg并不合适, 无任何对数据的更新/修改, 最终目的是想拿到img的拼接地址(或者默认图片)
形参res是一个对象, 实际使用到的属性只有两个基本类型, 遵循最少知识原则, 应修改形参.
let imgUrl, 其实是拼接url的前缀, 取名如 imgPrefixUrl会比较合适.
let 使用不当, 如 let imgUrl, 因涉及不到变量再赋值, 使用const是比较合适的
res.imgId, 即异常情况的处理, 少了对res.type的处理
变量命名, 如previewImgSrc, 取此名也不太合适, 该src使用的目的并不确定, 建议使用有比较通用涵义的名字. 可考虑重构为：

export function getImgSrc(imgId, imgType) {
    if(typeof imgId !== 'string' || typeof imgType !== 'string') {
        return require('@/asserts/images/icon.png')
    }
    return `${getImagePrefixUrl()}${imgId}.${imgType}?t=${+new Date()}`
}

前端如何去做CR（code review）