11 月 19 - 20 日 Apache Pulsar 社区年度盛会来啦,立即报名! 了解详情
写点什么

代码审查普遍存在的 6 大问题

  • 2020-04-09
  • 本文字数:1529 字

    阅读完需:约 5 分钟

代码审查普遍存在的6大问题

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


1、缺少变更的说明

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


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


2、滥用异常捕获

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


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


3、过度相信第三方

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


4、变量的作用域过大

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


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

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


6、日记打印问题

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


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


2020-04-09 16:431858

评论 3 条评论

发布
用户头像
感觉可以写下你们如何进行代码审查的,流程,规范,时间点
2020-04-13 16:26
回复
用户头像
"最近某宝弹窗事件导致其 APP 被大量用户删除,影响极其恶劣。" 这个被大量用户删除的数据哪里来的?
2020-04-09 19:27
回复
借楼说明一下,只针对这个事情背后表达观点,不对这个事情的数据和过程做展开
2020-04-13 15:12
回复
没有更多了
发现更多内容
代码审查普遍存在的6大问题_语言 & 开发_松花皮蛋me_InfoQ精选文章