code review应该怎么做

Code Review也就是代码评审。代码评审有两种不同的方法，一种是代码审查（比较正式），一种是代码走查（没那么正式），我们这里讨论的仅指代码走查。

之所以需要代码评审，是因为通常自己对自己写的代码都难以发现问题，因此需要以第二双眼睛再次检查代码，帮助我们及时地发现潜在的问题。

代码评审前的前提与准备工作

做代码评审之前，团队成员间需要达成一个共识，制定一份合理的代码规范标准。以此为前提，后续再补充，做代码评审的时候就会事倍功半，可以以下面为例：

1.Code Review不应该承担发现代码错误的职责。Code Review主要是审核代码的质量以及团队内部知识共享，而不是以缺陷和错误来批判他人，也不需要评论，表扬或是批评。评审的范围应限制在可读性，可维护性，以及程序的逻辑和对需求和设计的实现。代码中的bug和错误应该由单元测试，功能测试，性能测试，回归测试来保证（其中主要是单元测试，因为那是最接近Bug，也是Bug没有扩散的地方）。

2.Code Review不应该成为保证代码风格和编码标准的手段。代码规范与代码优化一定要区分开，不可相提并论。首先每个程序员的提交review的代码就应该是符合规范的，属于每个人自己的事情，不应该交由团队来完成。其次，作为一个审查者，你的任务不是确保被审查的代码都采用你的编码风格，因为它不可能跟你写的一模一样，而是要确保被审查的代码的正确性。

3.Code Review不应该仅仅只是走查，审查就完事了，还需要进行质量分析。比如CODING企业版产品就集成了代码评审和质量分析功能。

体系结构和代码设计的注意事项以及常见问题

检查代码是否被复用。如果代码被复制一次，虽然不喜欢这种方式，但通常没什么问题。但如果再一次被复制，就应该通过提取公共的部分来重构它，比如抽提出一个工具类什么的。

检查是否能用更好的代码。如果在一块混乱的代码做修改，添加几行代码也许更容易，但建议更进一步，用比原来更好的代码。比如Java编程新手有时候会检查new操作的结果是否为null。虽然说检查也没什么错误，但却不必要，因为在这里检查的代码唯一的功用是让整个程序更臃肿，运行更慢。

String str = new String("yanggb is a handsome boy");

if (str == null) { // 完全没有必要,new出来的对象不可能是null
    throw new RuntimeException("空对象");
}

用【==】替代【.equals】。在Java中，有两种方式检查两个数据是否相等：通过使用==操作符，或者使用所有对象都实现的.equals方法。因为原子类型（int、float和char等）不是对象，因此他们只能使用==操作符。

检查是否在catch块中作了清除工作。清除工作指的是清除无用的对象或释放资源，比如释放数据库连接资源。

检查是否有正确实现equals()，hashCode()，或者clone()等方法。虽然说这些方法由java.lang.Object提供的缺省实现是正确的，然而不幸的是，这些缺省实现在大部分时候毫无用处，因此许多类通过覆盖其中的若干个方法以提供更有用的功能。但是问题又来了，当继承一个覆盖了若干个这些方法的父类的时候，子类通常也需要覆盖这些方法。在进行代码审查时，应该确保如果父类实现了equals()，hashCode()，或者clone()等方法，那么子类也必须正确覆盖这些方法。

查找可能潜在的bugs。是否可能会引起其他的错误，比如循环是否以期望的方式终止。

检查错误处理。确定错误已经被优雅的修改了，而不会导致其他错误。

检查代码的执行效率。如果代码中包含算法，这种算法是否是高效？ 例如，在字典中使用迭代去遍历一个期望的值，就是一种低效的方式（使用map数据类型替代会高效很多）。

检查新代码与全局的架构是否保持一致。正常情况下，代码应该与全局的架构保持一致。比如检查基础代码是否有结合使用了一些标准或设计样式，新的代码是否遵循当前的规范，代码是否正确迁移，或参照了因不规范而淘汰的旧代码等。

检查代码的位置是否正确。比如涉及订单的新代码是否在订单服务相关的位置。

检查新代码是否被过度设计。大多数时候，过度设计并不是好事，可能使代码变得臃肿或执行效率下降。比如引入了现在还不需要的重用设计。

代码的可读性和可维护性

适当添加必要的注释。很多人图方便不写注释，自己方便了，可是别人看起来就费劲了。恰到好处的注释是很有必要的，第一，不能太多或太少，注释的形式根据代码具体的情况有不同；其次避免用注释包裹代码；最后是要尽量留下简明扼要的注释。

代码要写得清晰易懂。代码要尽量清晰地表达意图，比如写别人看得懂的单词，尽量使用语义化的命名组合，而不是像汉语拼音这样的命名。

String bianliang = "我是一个变量"; // 这样的代码太让人难过了

1.避免写一些现在不需要、将来也不太可能需要的功能。

2.字段、变量、参数、方法、类的命名是否真实反映它们所代表的事物, 是否能够望文生义?

3.避免大段代码，要写高内聚、低耦合的代码，能拆分的代码就拆分。

4.测试是否很好地覆盖了用例的各种情况？它们是否覆盖了正常和异常用例？是否有忽略的情况？

5.输出的错误信息是否可被理解？ log打的是否正确和足够?

6.不清晰的代码是否被文档、注释或容易理解的测试用例所覆盖？具体可以根据团队自身的喜好决定使用哪种方式。

7.简单就是美，避免简单的功能写出复杂的代码。

代码要灵活可变。不要把代码写死，预测可能发生的变化。比如一些通用的属性就应该放在字典库或枚举类中。

抽提复用的代码。通过提高代码的复用性提高代码的可维护性。

代码的功能检查

1.代码是否真的达到了预期的目标？如果有自动化测试来确保代码的正确性，测试的代码是否真的可以验证代码达到了协定的需求？

2.代码看上去是否包含不明显的bug，比如使用错误的变量进行检查，或误把and写成or？

3.是否有会在生产环境中导致应用停止运行的明显错误？代码是否会错误地指向测试数据库，是否存在应在真实服务中移除的硬编码的stub代码？

4.你对性能的需求是什么，你是否考虑了安全问题？

5.这个新增或修复的功能是否会反向影响到现存的性能测试结果。

6.外部调用很昂贵（a. 数据库调用. b. 不必要的远程调用. c. 移动或穿戴设备过频繁地调用后端程序）。

代码的安全问题

代码的常规审查不可少，安全审查也不可少，对安全性要求较高的程序尤其要注意。如果缺少了这道流程，万一遭受攻击，带来的损失将远超过我们的想象，包括识别威胁，检查是否存在常见安全漏洞，以及遭受安全威胁时如何应对和补救等，这是一个很打的话题，这里就不做展开。

1.识别可能受到的威胁。STRIDE可用来识别对上述元素的威胁。STRIDE，是【假冒身份、篡改数据、否认、信息泄露、拒绝服务和权限提升英文单词】的缩写。

2.检查是否新的路径和服务需要认证。

3.检查数据是否需要加密。

4.检查密码是否被很好地控制。这里的密码包含密码（如用户密码、数据库密码或其他系统的密码）、秘钥、令牌等等。这些永远不应该存放在会提交到源码控制系统的代码或配置文件中，有其他方式管理这些密码，例如通过密码服务器（Secret Server）。当审查代码时，要确保这些密码不会悄悄进入你的版本控制系统中。

5.讨论代码的运行是否应该被日志记录或监控。日志和监控需求因各个项目而不同，一些需要合规，一些拥有比别人严格的行为、事件日志规范。如果你有规章规定哪些需要记录日志，何时、如何记录，那么作为代码审查者你应该检查提交的代码是否满足要求。如果你没有固定的规章，那么就考虑：

• 代码是否改变了数据（如增删改操作）？是否应该记录由谁何时改变了什么？
• 代码是否涉及关键性能的部分？是否应该在性能监控系统中记录开始时间和结束时间？
• 每条日志的日志等级是否恰当？一个好的经验法则是【ERROR】触发一个提示发送到某处，如果你不想这些消息在凌晨3点叫醒谁，那么就将之降级为【INFO】或【DEBUG】。当在循环中或一条数据可能产生多条输出的情况下，一般不需要将它们记录到生产日志文件中，它们更应该被放在【DEBUG】级别。

其他方面

1.是否存在内存无限增长? 例如，如果审查者看到不断有变量被追加到list或map中，那么就要考虑下这个list或map什么时候失效，或清除无用数据。

2.代码是否及时关闭了连接或数据流?

3.资源池配置是否是否正确? 有没有过大或者过小?

4.调用接口出错的时候, 是否有出错处理逻辑, 并且处理正确?

5.进程意外重启后, 是否能够恢复到崩溃前的环境?

6.正确性(主要与多线程环境关系密切)

7.代码是否使用了正确的适合多线程的数据结构

8.代码是否在不需要的地方使用同步或锁操作？如果代码始终运行在单线程中，锁往往是不必要的

9.条件判断语句或其他逻辑是否可以将最高效的求值语句放在前面来使其他语句短路？

10.代码是否存在许多字符串格式化？是否有方法可以使之更高效？

11.日志语句是否使用了字符串格式化？是否先使用条件判断语句校验了日志等级，或使用延迟求值？

Code Review的实际操作建议

1.代码审查是应该在互相沟通中进行讨论的，而不是相互对抗。预先确定哪些是要点哪些不是，可以减少冲突并拟定预期。

2.要经常进行Code Review，不要攒了1w行才让同事帮你review，这是坑队友。

• 要Review的代码越多，那么要重构，重写的代码就会越多。而越不被程序作者接受的建议也会越多，唾沫口水战也会越多。
• 程序员代码写得时候越长，程序员就会在代码中加入越来越多的个人的东西。 程序员最大的问题就是【自负】，无论什么时候，什么情况下，有太多的机会会让这种【自负】蔓延开来，并开始影响团队影响整个项目，以至于听不见别人的建议，从而让Code Review变成了口水战。
• 越接近软件发布的最终期限，代码也就不能改得太多。

3.Code Review要简短、轻量，不要太正式。

• 平时工作量也比较忙，评审时间太长也会影响工作进度，造成延期交付，得不偿失。
• 就算review时间没有造成什么影响，增加review时间也不会明显增加发现问题数量。
• 只用让代码审查简短、轻量，才能有效的发现问题，这样更适合迭代、增量开发，为开发者提供更快的反馈。

4.减少代码审查人数量，且让不同人review，建议三人组。

• 并不是代码审查人员越多就能发现越多Bug，第一个人审查完后，第二个人发现的Bug仅为剩下问题的一半，再多个人发现问题数量也没有明显变化，所以一般不超过三人。
• 更多代码审查人员意味着多人在寻找同样的问题，造成重复工作量，另外由于指望其他人员，使得审查人员积极性、主动性不高，更加不利于代码审查工作的有效进行。
• 三个人我认为是最合适最高效的数量，可以从不同的方向评审。保持积极的正面的态度。

代码审查工具

通常，代码审查工具大致分为两类，一类是按照预先定义号的规则来审查代码，自动执行并生成报告，另一种是合作共同检查和讨论变更的场景，而且需要将这过程的历史也存储下来以备将来参考。需要说明的是，没有最好的工具，只有适合自己的工具，适合就是最好，请大家根据自己的项目情况来做出选择。

总结

代码评审对于代码作者，审查人以及团队都是有益的，经常阅读自己代码并修改重构自己代码的习惯，因为编写者都会觉得自己代码写的很完美，这是正常的现象。不过如果你过段时间再次看自己当初以为写的很牛的代码可以发现好多吐槽点，好多可以优化重构的地方，保持这种经常阅读自己代码并重构的习惯可以提高自己的代码能力。也可以经常阅读别人的代码看别人的代码风格有何借鉴之处，正所谓三人行必有吾师。

 

"人生不可逆，还是要试着去看看更美的风景。"