导读

最近两年，品质中心极力推动测试工作左移，以期能提前发现产品的问题，降低成本。笔者自认代码基础能力还不错，就想通过代码 Review 来提前发现一些 Bug。

多数项目中，代码评审工作是由开发同事相互执行的。但往往开发同事为了赶进度，并没有时间进行代码评审，导致很多明显的 Bug 被遗留到了测试阶段。那代码评审是否可以由测试人员来做呢？显然是可以的。诚然多数测试人员的代码能力没有开发人员的水平，代码 Review 的深度不如开发同事，但通过实践证明，测试人员也能胜任大部分代码评审的工作。

做 CodeReview 的方法

笔者在刚开始做代码 Review 时也是毫无头绪，不知道哪些代码可能有问题。那时我才意识到了解 Bug 出现的根因对代码 Review 有至关重要的作用。

通过对 Bug 及开发对应修改的代码进行分析，并与开发同事交流，我了解到一些 Bug 出现的原因，以及出错代码的一些特征。当这些代码特征被总结出来后，我将这种特征用于 Review 其他的代码，此时能慢慢地能发现一些 Bug 了，但效率比较低。

后来用 Android Studio 自带的 Lint 工具扫描代码可以扫描出大量疑似缺陷的点，再通过人工分析可以发现不少空指针和逻辑上的问题，Review 代码的效率得到了极大的提升。

但还有一些更深层次的需求，比如像一些多条件组合的代码就不能通过 Lint 扫出来了。因此我把这些特殊的代码特征进行汇总，请一个同事帮忙写了一个定制化的代码扫描工具，利用这个工具扫描出代码位置，然后针对性的 Review。

总结我的实践过程，建议刚开始做代码 Review 的朋友，先使用一些业界常用的工具快速入手。当积累一些经验后，尝试自己分析问题并总结经验，好的经验积累起来形成自己的知识库和工具库，提升 Review 效率。

Review 知识点汇总

以下是笔者在平时工作中总结出一些经常可以发现问题的点，希望对同仁们有所帮助。

1、空指针

如果项目有异常上报统计，就会发现最常见的异常是空指针异常 (NullPointerException),代码中如果使用了未初始化的对象都会导致这个异常。一般开发都会在程序入口处进行参数的判空，不过这样还不够。严格意义上，任何一个对象在使用前都应该进行判空处理。

如下代码片断所示，一些开发同事习惯当传入参数为空时，直接返回一个空的对象。单从本方法的角度来看是不会有问题的，但是在调用本方法的地方，如果忘记做判空处理就会出现空指针的错误。

以上示例中较好的代码实践是返回一个没有元素的列表，或者是当参数为空时直接显式的抛出一个异常，让调用者必须处理该异常。

针对空指针的情况，一般 Review 以下几点：

（1）方法参数如果不能为空时，是否做了判空处理，或者在方法调用者传入参数时是否确保了不为空；

（2）方法是否有返回 null 的情况，如果有是否可以改为返回一个空白对象（如没有元素的列表等）；

（3）当被调用的方法 (如系统方法) 返回为 null 时，调用者是否有进行判空处理；

（4）使用的对象是否在使用时已经被初始化。较常见出现问题的情况是类的成员，如果在构造函数中没有进行初始化，而在其他地方进行初始化时，初始化时机是未知的，那么此时对象使用前一定要进行判空。

2、逻辑判断

（1）边界判断

数组越界 (OutOfBoundaryException) 在异常统计上报中也是比较常见的问题,这是最常见的一种边界条件不正确引起的问题。

数组或者列表边界一般 Review 的点有以下几个：

1）数组或列表的循环中，合法下标范围是 0<=K<list.size()；

2）通过下标从数组或列表取数据时，下标不合法的判断方法是 if (k < 0 || k >= list.size())；

3）当在下标存在加减时,需要判断当加上或减去某值后,是否可能存在越界的情况；

4）如果是分隔字符串产生的数组，取数组的值前一定要判断下标是在数组长度范围内的；

5）取数组或列表的项时，需要首先判断数组或列表的长度不为 0。

（2）逻辑判断

任何一个 if 语句都有两个分支。当仅有一个 if 时，开发一般不会漏掉 if-else 两个分支。

但如下面的示例代码，本身可能不存在问题。但可以看出组合起来的条件分支会有很多，当 if-elseif-else 组合嵌套时，开发同事会重点关注满足需要条件的情况，却往往容易忽略 else 应该做的处理。像以下的示例代码，也要思考是否能将判断条件组合来用，减少嵌套。

另外多条件组合的判断逻辑，特别是判断条件超过两个时，或者是 “&&” 与 “||” 组合使用时也非常容易出错。

如下的示例代码，首先这段代码不容易理解，看到这段代码时需要想想 “&&” 与 “||” 哪个的优先级高，如果用括号包起来就会更容易理解；其次经过详细分析后发现最终结果与 isCacheCurrentChapter 的值无关。

又如下面的示例代码，doSomething 的方法接受的参数不为空，然而当 a 的值为空时会中断后续判断逻辑,b 即使为空也会传入到 doSomething 方法中，导致 doSomething 不能正常运行。

因此，对于以上类似的判断逻辑代码，可以做的评审有三点:

1）是否能优化判断逻辑，使代码更加简洁易懂；

2）是否所有的分支都得到了合理的处理，如代码中没有写出来的 else 分支，或者 Switch 的 default 分支；

3）是否存在条件判断的中断情况，对后续一些判断或者逻辑造成影响。

3、函数中途返回

函数中途返回指在运行过程中, 达到了某种条件, 使程序中途 return 的情况。

如下面的代码所示，当 info 为空时直接返回了，乍一看似乎没有任何问题；但如果认真地思考后，会发现 container 对象还在等待一个回调，Review 时需要去检查没有执行这个回调方法是否会存在问题。

因此针对类似的在中途返回的情况，Review 时需要看看是否存在 return 导致某些逻辑不能正确执行到的情况。

4、内存泄漏

当程序偶尔出现莫名其妙的卡顿或异常，又或者 Crash 上报出现 OOM 异常时，那作为测试人员就该意识到程序有内存泄漏了。

内存泄漏除了通过专门的测试方法来测试外，也可以通过代码 Review 来发现。

对 QQ 浏览器的内存泄漏测试发现的 Bug 原因分析，发现导致内存泄漏最频繁的原因不是图片资源或者 IO 流 (Stream) 未释放，而是注册了事件未取消注册引起的内存泄漏。

如下面的示例代码所示，FooActivity 将自己注册到了 FooDataManager，便于在数据发生变化时自己能收到通知。

如下面的代码所示，FooDataManager 一般都会用一个列表来存储注册的监听者，如果 FooDataManager 需要运行很长时间甚至整个生命周期，或者 listener 本身是一个静态对象的话，那么 listener 会长期存在于内存中，这意味着 listener 中存放的对象也会被长期持有，最终导致内存泄漏。

前面示例中的 FooActivity 并未将自己反注册，listener 一直持有该对象造成内存泄漏。

以上问题看起来似乎很简单，但是在浏览器项目中，即使高级的开发工程师也会犯类似的错误。当然内存泄漏的原因还有很多，这里就不全部列举了，大家可以网上搜索进行了解。

针对内存泄漏的情况，我一般会 Review 以下几种常见情况：

（1）对象如果注册了事件回调，是否在合理的地方进行了反注册；

（2）线程对象使用完毕是否正常的结束；

（3）各种数据库、网络连接和文件 IO 被打开后，是否正确关闭；

（4）图片资源正确释放；

（5）缓存对象要有一定的大小控制，且有明确的释放策略。

5、异常处理

关于异常处理的评审，笔者一般会关注当异常被捕获后，是否正确的处理，以及当有异常处理后，后续的流程是否正常执行。

如下面的代码所示，当 catch 到异常时，此时 looper 是为空的，到后续的 Handler 初始化传入空的 looper 程序会出错。

效果

代码评审在 QQ 浏览器漫画模块最近了三个版本进行了实践，共发现 Bug25 个，如下面的截图所示。由于代码 Reviwe 在开发阶段就进行，Bug 发现的时间提前了至少一周。

总结

以上是我的一点经验总结，还需要持续积累。

万事开头难，个人以为做代码 Review 在刚开始的时候会稍微难一些，但只要做到以下几点一定能做好代码 Review。

第一，学会使用一些业界比较常用的代码扫描工具，可以快速入手；

第二，坚持学习提升自己的代码能力，并掌握快速阅读和理解代码的方法；

第三，加深对自己产品的业务和代码结构的理解，更容易发现深层问题。

最后，学会通过 Bug 根因分析，总结经验并应用于平时的工作中。

以上内容分享给大家，与大家共勉，希望我们一起进步！

关注微信公众号：腾讯移动品质中心 TMQ，获取更多测试干货！