代码审查常见问题，建议收藏

原创松花皮蛋松华说

最近某宝弹窗事件导致其APP被大量用户删除，影响极其恶劣。我在想，如果他们的内部代码审查更加严格一点，少走形式，就能将隐患扼杀在摇篮里了。基于此，我们部门专门成立了由小组长和核心成员组成的代码评审组，将以前的代码评审模式进行了一些优化，加强了管控。我在以往代码评审过程中发现了一些普遍存在的问题，总结如下。

1、缺少变更的说明

通常情况下我不写注释的原因只有一个，这几行代码过于简单，其意思可由代码翻译得到，但是有些人写的代码里通篇没有注释，我就很纳闷了，难道是我太菜了嘛？

说实在的，注释和变更说明真的是一对难兄难弟，我们经常对他们视而不见，完全没有在开源框架中注意到他们。其实不难发现框架的代码注释和变更说明是非常多的，包括使用示例、开始支持的版本号等等，同时也不难发现我们的代码库提交变更注释满屏都是"update"，满怀期待地打开下一页，终究还是被打脸了。或许背后是规范问题，或许是因为我们还没有开始问候前人。

2、滥用异常捕获

我在不少项目中发现一些奇怪的骚操作，每个控制器方法都使用了全局的try-catch异常捕获，然后在其catch方法中处理可用率下降埋点。编程容易产生模板化思维，可能自己不觉得有什么不妥，但是这种方式实在不优雅。我们完全可以使用全局切面进行统一异常处理，根据返回的错误码进行可用率下降埋点。假如控制器方法返回不是统一的规范结果对象，那就停下手中的工作开始重构吧！另外优化前的方法，很容易让我们对中间结果放松警惕，毕竟有这么大而全的异常捕获，是不是非空判断或者非法判断统统都可以省略了？！

上面说的问题完全不会影响代码的运行，但是场景切换到事务操作，就不得不小心了，因为生吞异常会导致事务回滚没有机会执行。所以，如果调用者关心异常，就尽情往上层抛吧！

3、过度相信第三方

过度相信第三方的意思，包括前端传过来的参数没有进行校验、调用上游接口返回的结果没有进行判断、不进行参数校验就调用其他团队提供的写数据接口，最后一个示例有可能导致SQL漏洞攻击，虽然主要责任不是自身。不过，我在真实开发中就因为过度相信第三方差点体验了一把删库跑路，我从类中拷贝了几行代码，当参数非空时就执行更新操作，但是当前端没有传该参数时，该参数类型为非包装类也就是默认非空，从而覆盖数据库记录。这种错误也是新手处理空值判断容易犯的。

4、变量的作用域过大

我们会不经意间开始随意玩弄变量的作用域，比如把变量的创建和赋值操作分别由两个不同的方法实现，显然给系统增加了不确定性。另外，变量太多容易淹没主干逻辑。如果它们有联系或者会产生协作，应该共用一个方法专门管理。

5、处理过程缺少阶段性结果

没有程序员喜欢阅读大量的判断语句，而提前异常判断快速返回结果是一个基本的优化方法，比如当校验不合法时直接返回，再比如移除控制标记，来减少循环嵌套。语义再拔高点不就是处理过程要有阶段性结果了嘛，这样代码更加有层次感、整体流程更加清晰。

6、日记打印问题

我相信大部分公司都采用了微服务的架构，这种架构的难点之一就是问题的快速排查，特别是涉及到跨团队。而封装日记信息就是我们唾手可得的利器，记录参数、中间结果、返回结果、异常信息，有了这些信息后，找上游反馈问题就更加理直气壮了，而不需要修改代码添加日记后重新上线。所以，尽量多记录一些异常信息，尽量多添加一些INFO级别的日记。不过，日记记录毕竟是旁路逻辑，因为记录而产生的空指针异常或者内存溢出就得不偿失了，所以记录时要避免序列化空对象，避免使用无界的异步队列。

到这里本文就要结束了，总结一下，我在文中和你分享了六种容易犯的错误，包括缺少变更的说明、滥用异常捕获、过度相信第三方、变量的作用域过大、处理过程缺少阶段性结果、日记打印问题。希望对你有所帮助，欢迎分享。

继续滑动看下一个