1、什么是CodeReview？

Code Review（CR）即代码评审，又名代码走查，是一种通过复查代码来提高代码质量的过程，一般体现在一个团队的开发过程中。CR要求团队成员有意识地、系统地检查彼此的代码，从而验证需求、发现错误，同时指出其中不合规范的“低质量”代码，从而提高整个团队的代码质量。

一次 CR 可以是一次 Commit，也可以是一次 Merge Request。因此，实践课系统支持团队内部的 MR 评审以及 Commit 评审，供大家学习和交流。

2、为什么要CodeReview？

（1）旁观者清。

• 对于同一段业务代码，由于看待问题的角度不同，评审者可能会比开发者更容易发现其中的问题，或是找到更有效的解决方案，共同维护团队的代码质量。
• 提高代码质量和可维护性, 可读性等。
• 查漏补缺, 发现一些潜在的问题点等。
• 最佳实践, 能够更好更快的完成任务的方法。
• 知识分享, Review他人代码时, 其实也是一个学习的过程, 自己也会反思&总结。

（2）快速了解业务。

• 理想状态下，团队中的每个人都需要对整个项目的各个部分都很熟悉，当然，在项目很大时这是不现实的。通过代码审查至少可以让每个人了解更多的业务模块，同时也能达到人员互备的目的。
• 人员互备：通过 CR，评审者也相当于参与了这次开发，相当于一种人力“备份”，当你休假或正在忙别的需求的时候，这时“备份”或许就能帮上你的忙了。

（3）开发者能够获得什么？

• 对需求的理解得到加深。
• 表达能力得到加强。
• 逻辑能力得到训练。
• 心理承受能力得到提高。

（4）评审者能够获得什么？

• 快速上手业务需求和全局的架构。
• 统一大家约定俗成的代码风格。
• 优秀的设计思路和业务逻辑。

3、CodeReview 的原则。

（1）CR 是必要的，但也需要结合团队现状。

• 当你的团队开发任务极其紧张，再耗费一部分人力去进行 CR，是不明智的。

（2）所有的代码都应被赞成。

• 因为团队代码库的每一次改动（Change List ，以下简称 CL），都必定会提高当前系统的整体质量，即使这个 CL 并不完美。

（3）CR 不应该追求完美，而应追求持续改进。

• 要知道，没有完美的代码，只有更好的代码，“慢即是快”。

（4）CR 不是挑刺，更不是证明谁的能力更强。

• CR 是为了提高整个团队的能力，而不是针对个体设置的检查“关卡”，仅具有指导意义。

（5）评审者也需要对这个 CL 负责。

• CR 不应设置奖惩机制，即便是有，也是对评审者和开发者同时的奖励或处罚。

（6）时刻保持谦虚。

• 无论是评审者还是开发者，都可以在 CR 中提升自己的能力。

（7）合理解决问题

• 解决冲突难以达成共识时，需要面对面或者拉起更大的团队讨论，带上leader。

4、在一次CodeReview中我需要关注什么？

（1）git 提交规范

（2）代码风格

a. 可读性

• 衡量可读性, 有很好的实践标准, 即 Code Review 时能否非常容易的理解代码逻辑, 如果不能, 那意味着代码的可读性要进行改进。

b. 命名

• 描述是否准确。不建议使用生僻单词。
• 命名格式是否与团队风格一致。

c. 函数体长度/类长度

• 函数体太长，不利于阅读。一般建议不超过50行。
• 类太长，有可能违背“单一职责原则”。

d. 参数个数

• 参数不要太多。一般不超过5个；5个以上建议使用对象。

e. 注释

• 恰到好处的注释，能够帮助评审者更好地理解函数体和类。

（3）架构/设计

a. 单一职责原则

• 一个类只做一类相关的事情。
• 一个方法，最好只做一件事情。

b. 行为是否统一

• 校验处理是否统一。
• 数据处理是否统一。
• 错误处理是否统一。
• …

c. 代码是否污染

• 代码是否对其他模块有强耦合。

d. 是否有重复代码

• 检查是否将可复用的方法或组件抽取出来。

e. 开放-封闭原则

• 代码是否具有良好的可扩展性。

f. 代码健壮性

• 核心数据有没有强制校验。
• 边界值有没有考虑得当。
• 有没有潜在的bug。
• 有没有内存泄露。
• 有没有循环依赖。
• …

g. 是否考虑了错误处理

• 有没有很好的 Error Handling。

h. 面向接口编程

• 检查业务的实现中有没有进行合适的抽象。

i. 效率/性能

• 对用户使用频率高、资源消耗大的业务部分是否处理得当。
• 关键算法的时间复杂度。
• 有没有潜在的性能瓶颈。

j. 代码重构

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

k. 时间粒度。

• 一个Review单元的时间应控制在10~20分钟，如果超出这个时间，那么这段代码本身可能在业务划分或代码实现上，是存在问题的。