代码审查普遍存在的问题

 

本文总结了代码评审过程中普遍存在的一些问题,具体如下:

1、缺少注释与变更的说明

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

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

2、滥用异常捕获

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

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

3、过度相信第三方

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

4、变量的作用域过大

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

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

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

6、日记打印问题

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

—— 完 ——
相关推荐
评论

立 为 非 似

中 谁 昨 此

宵 风 夜 星

。 露 , 辰

文章点击榜

细 无 轻 自

如 边 似 在

愁 丝 梦 飞

。 雨 , 花