接手前同事代码,我差点吐了
The following article is from yes的练级攻略 Author 是Yes呀
年前公司优化了一些人,最近我开始接手他们的项目。
然后...我就麻了。
一开始接到这个任务的时候,我没怎么当回事儿,根据一些需求评估的时间也比较紧凑。
后面越看越不对劲,估时两天的开发,我每天加班都硬生生的花了 5 天。
绝大部分业务实现就是直接干,硬怼!
没有抽象,没上缓存,没有批处理的概念循环一个一个调,有些方法也没有事务,更别说补偿。
一个方法多的有七八百行,平铺直叙。
上传图片、创建订单等等操作一个方法里面同步直接干,超不超时就随缘。
把我都看傻了,也得亏这个项目是新项目,当前没什么用户,不然真有用户大量使用这方法不报错我直播倒立洗头。
鉴赏下
业务或者性能方面不说了,因为就连编码风格或者说基础可能还不如实习生(我们公司都是招5年以上的)。
我截了几处,一起来来鉴赏下:
都是公司代码,所以打码
先来个开胃菜,热热场子:
这样的代码很正常,有些人写 demo 会输出到控制台,但是对应生产项目里面在 main 分支我看到很多这样的代码,这就有点说不过去了。
就算习惯用这种方式调试,一两个可以说忘了删了,但是遍地都是说明压根就没想删除。
对自身提交代码的要求太低了。(合并分支的同学也得背点责任)
趁热打铁,我们再来看下这段代码:
JSONObject 本身就实现了 Map 接口,内部也是个 map:
也就是说 JSONObject 本身就可以通过 key 去 get value,何必再变成个 map 执行一样的操作,em....。
上述的代码可以说其实影响不大,接下来的代码就有点东西了!
来看看这个 sleep:
首先看下这个方法的上面标记了 @Async 说明这是一个异步方法,那为什么要 Thread.sleep(300)
?
起初我也没看明白,后来发现了这个异步的方法里面需要修改前面主方法新增的一个数据库数据,所以这里 Thread.sleep(300)
的目的就是等待主线程的事务提交,不然异步方法就取不到数据了。
我称这个为玄学编程,就是拿 300 毫秒赌主线程的事务已经提交,提交了好说,正常执行异步的逻辑。
要是没提交?那就....爱咋咋了。
既然说到 @Async 了,那继续看这个项目里的另一个 @Async 使用,我称之为假想异步:
可以看到声明了一个 createWxxx
方法,使用 @Async,很明显编码者想异步执行这个方法里面的内容。
然后我查看这个方法的引用,发现全局只在同个类里面的另一个方法使用:
就是个类的本地方法调用。
我们都知道在 Spring 里面,不论是事务注解还是异步注解,通通都是依靠 AOP 注入逻辑的,而内部方法调用是走不了代理的,所以这里的 @Async 压根就没生效,所以说这叫假想异步:只要我加了 @Async 我就异步啦!(还指定了线程池)
最后们再看一个分布式锁的操作。
我先给你点时间仔细看看下方的代码是否有问题。
其实一开始我都没发现,后面定睛一看,我滴妈加解锁的 key 都不一样,可以看到加锁的 key 比解锁的 key 多个 dto.getOrderNo()....
ok,没问题,这可以认为是疏忽,谁都有疏忽的时候。
但是分布式锁是这样用的??
if (redis.setNx(xxx)) {
//抢到锁了,dosth
}
finally {
redis.del(xxx) ???
}
如果抢到锁就执行后面逻辑,这个没毛病,但是 finally 里面直接就删除锁是个什么意思??
合着抢不到锁,也直接把别人的锁给删了?
这还加什么锁呢?
这压根就是个大坑,坑的后面的人可能需要疯狂修数据!
最后
哎,还有很多神操作,比如 for 循环 RPC 调用外面还套了个事务。。真的都是大改!
其实很多时候公司压根不需要什么多牛的程序员。
基础踏实,稍微有点代码追求的程序员在大部分情况下比大牛都重要多了,毕竟一个项目绝大部分代码都是普通程序员写的,如果这里一点坑那里一点坑,加起来就是屎山,坑的就是公司和后人。
<END>
程序员专属T恤