代码review,瑞出事来了!

快来打我* 2023-09-24 00:30 177阅读 0赞

不久之前,部门进行了一次代码评审。

代码整体比较简单,该吹B的地方都已经吹过了,无非是些if else的老问题而已。当翻到一段定时任务的一步执行代码时,我的双眼一亮,觉得该BB两句了。

谁知这群家伙,评审的时候满满的认同感,但评审结束不久,就给我冠了个事B的称号。

今天我就把当时的这些话儿整理整理,让大家说道说道,我到底是不是个事B。淦!

一个任务处理例子

代码的结构大体是这样的。

通过定时,这段代码每天晚上凌晨都要对数据库的记录进行一遍对账。主要的逻辑,就是使用独立的线程,渐进式的读取数据库中的相关记录,然后把这些记录,放在循环中逐条进行处理。

  1. ExecutorService service = Executors.newFixedThreadPool(10);
  2. ...
  3. service.submit(()->{
  4. while(true){
  5. if(CollectionUtils.isEmpty(items)){
  6. break;
  7. }
  8. List<Data> items = queryPageData(start, end); // 分页逻辑
  9. for(Data item : items){
  10. try {
  11. Thread.sleep(10L);
  12. } catch (InterruptedException e) {
  13. //noop
  14. }
  15. processItem(item);
  16. }
  17. }
  18. });
  19. 复制代码

等一下。在代码马上被翻过去的时候,我叫停了,这里的processItem没有捕获异常

通常情况下,这不会有什么问题。但静好的岁月,总是偶尔会被一些随机的事故打断。如果这是你任务的完整代码,那它就有一种非常隐晦的故障处理方式。即使你的单元测试写的再好,这段代码我们依然可以通过远程投毒的方式,通过问题记录来让它产生问题。

是的。以上代码的根本原因,就是没有捕捉processItem函数可能产生的异常。如果在记录处理的时候,有任何一条抛出了异常,不管是checked异常还是unchecked异常,整个任务的执行都会终止!

不要觉得简单哦,踩过这个坑的同学,请记得扣个666。或者翻一下你的任务执行代码,看看是不是也有这个问题。

Java编译器在很多情况下都会提示你把异常给捕捉了,但总有些异常会逃出去,比如空指针异常。如下图,RuntimeException和Error都属于unchecked异常。

format_png

RuntimeException可以不用try…catch进行处理,但是如果一旦出现异常,则会导致程序中断执行,JVM将统一处理这些异常。

你捕捉不到它,它自然会让你的任务完蛋。

如果你想要异步的执行一些任务,最好多花一点功夫到异常设计上面。在这上面翻车的同学比比皆是,这辆车并不介意再带上你一个。

评审的小伙很谦虚,马上就现场修改了代码。

不要生吞异常

且看修改后的代码。

  1. ExecutorService service = Executors.newFixedThreadPool(10);
  2. ...
  3. service.submit(()->{
  4. while(true){
  5. if(CollectionUtils.isEmpty(items)){
  6. break;
  7. }
  8. List<Data> items = queryPageData(start, end); // 分页逻辑
  9. for(Data item : items){
  10. try {
  11. Thread.sleep(10L);
  12. } catch (InterruptedException e) {
  13. //noop
  14. }
  15. try{
  16. processItem(item);
  17. }catch(Exception ex){
  18. LOG.error(...,ex);
  19. }
  20. }
  21. }
  22. });
  23. ...
  24. service.shutdownNow();
  25. 复制代码

为了控制任务执行的频率,sleep大法是个有效的方法。

代码里考虑的很周到,按照我们上述的方式捕捉了异常。同时,还很贴心的把sleep相关的异常也给捕捉了。这里不贴心也没办法,因为不补齐这部分代码的话,编译无法通过,我们姑且认为是开发人员的水平够屌。

由于sleep抛出的是InterruptedException,所以代码什么也没处理。这也是我们代码里常见的操作。不信打开你的项目,忽略InterruptedException的代码肯定多如牛毛。

此时,你去执行这段代码,虽然线程池使用了暴力的shutdownNow函数,但你的代码依然无法终止,它将一直run下去。因为你忽略了InterruptedException异常。

当然,我们可以在捕捉到InterruptedException的时候,终止循环。

  1. try {
  2. Thread.sleep(10L);
  3. } catch (InterruptedException e) {
  4. break;
  5. }
  6. 复制代码

虽然这样能够完成预期,但一般InterruptedException却不是这么处理的。正确的处理方式是这样的:

  1. while (true) {
  2. Thread currentThread = Thread.currentThread();
  3. if(currentThread.isInterrupted()){
  4. break;
  5. }
  6. try {
  7. Thread.sleep(1L);
  8. } catch (InterruptedException e) {
  9. currentThread.interrupt();
  10. }
  11. }
  12. 复制代码

除了捕捉它,我们还要再次把interrupt状态给复位,否则它就随着捕捉给清除了。InterruptedException在很多场景非常的重要。当有些方法一直阻塞着线程,比如耗时的计算,会让整个线程卡在那里什么都干不了,InterruptedException可以中断任务的执行,是非常有用的。

但是对我们现在代码的逻辑来说,并没有什么影响。被评审的小伙伴不满意的说。

还有问题!

有没有影响是一回事,是不是好的习惯是另一回事 。我尽量的装了一下B,其实,你的异常处理代码里还有另外隐藏的问题。

还有什么问题?,大家都一改常日慵懒的表情,你倒是说说

format_png 1

我们来看一下小伙伴现场改的问题。他直接使用catch捕获了这里的异常,然后记录了相应的日志。我要说的问题是,这里的Exception粒度是不对的,太粗鲁。

  1. try{
  2. processItem(item);
  3. }catch(Exception ex){
  4. LOG.error(...,ex);
  5. }
  6. 复制代码

processItem函数抛出了IOException,同时也抛出了InterruptedException,但我们都一致对待为普通的Exception,这样就无法体现上层函数抛出异常的意图。

比如processItem函数抛出了一个TimeoutExcepiton,期望我们能够基于它做一些重试;或者抛出了SystemBusyExcption,期望我们能够多sleep一会,给服务器一点时间。这种粗粒度的异常一股脑的将它们捕捉,在新异常添加的时候根本无法发现这些代码,会发生风险。

一时间会议室里寂静无比。

我觉得你说的很对 ,一位比较资深的老鸟说, 你的意思是把所有的异常情况都分别捕捉,进行精细化处理。但最后你还是要使用Exception来捕捉RuntimeException,异常还是捕捉不到啊

果然是不同凡响的发问。

优秀的、标准的代码写法,其中无法实施的一个重要因素,就是项目中的其他代码根本不按规矩来。如果我们下层的代码,进行了正确的空指针判断、数组越界操作,或者使用类似guava的Preconditions这类API进行了前置的异常翻译,上面的这种问题根本不用回答。

但上面这种代码的情况,我们就需要手动的捕捉RuntimeException,进行单独的处理。

你们这个项目,烂代码太多了,所以不好改。我虽然有情商,但我更有脾气。

大家不欢而散。

End

我实在是想不通,代码review就是用来发现问题的。结果这review会一开下来,大家都在背后讽刺我。这到底是我的问题呢?还是这个团队的问题呢?让人搞不懂。

你们在纠结使用Integer还是int的时候,我也没说什么呀,现在就谈点异常处理的问题,就那么玻璃心受不了了。这B不能全都让你们装了啊。

什么?你要review一下我的代码?看看我到底有没有像我说的一样写代码,有没有以身作则?是在不好意思,我可是架构师哎,我已经很多年没写代码了。

你的这个愿望让你落空了!

format_png 2

作者简介:小姐姐味道 (xjjdog),一个不允许程序员走弯路的公众号。聚焦基础架构和Linux。十年架构,日百亿流量,与你探讨高并发世界,给你不一样的味道。我的个人微信xjjdog0,欢迎添加好友,进一步交流。

作者:小姐姐味道

原文链接:https://juejin.cn/post/7080155730694635534

发表评论

表情:
评论列表 (有 0 条评论,177人围观)

还没有评论,来说两句吧...

相关阅读

    相关 如何做代码Review

    为什么要做代码Review? 提高代码质量,提升自身水平 及早发现潜在缺陷与Bug,,降低事故成本 促进团队内部知识共享,提高团队整体水平 保证项

    相关 代码review

    不久之前,部门进行了一次代码评审。 代码整体比较简单,该吹B的地方都已经吹过了,无非是些if else的老问题而已。当翻到一段定时任务的一步执行代码时,我的双眼一亮,觉得该B

    相关 代码review

    不久之前,部门进行了一次代码评审。 代码整体比较简单,该吹B的地方都已经吹过了,无非是些if else的老问题而已。当翻到一段定时任务的一步执行代码时,我的双眼一亮,觉得该B

    相关 代码review

    不久之前,部门进行了一次代码评审。 代码整体比较简单,该吹B的地方都已经吹过了,无非是些if else的老问题而已。当翻到一段定时任务的一步执行代码时,我的双眼一亮,觉得该B

    相关 Code Review 代码规范

    1.编码常规项 代码是否内存泄漏,是否UI成红色,是否性能低下,是否会形成crash 各种异常逻辑是否处理 代码能够工作么?它有没有实现预期的功能,逻辑是否

    相关 代码Review那些

    > 本篇推文是以前同事做分享的时候的ppt,这里我整理出来分享给大家 什么是代码Review? 代码review是指在软件开发过程中,通过对源代码进行系统性检查来确认代

    相关 代码Review那些

    > 本篇推文是以前同事做分享的时候的ppt,这里我整理出来分享给大家 什么是代码Review? 代码review是指在软件开发过程中,通过对源代码进行系统性检查来确认代