代码 review，瑞出事来了 - 今日头条

本文由 简悦 SimpRead 转码， 原文地址 www.toutiao.com

不久之前，部门进行了一次代码评审。代码整体比较简单，该吹 B 的地方都已经吹过了，无非是些 if else 的老问题而已。

原创：小姐姐味道（微信公众号 ID：xjjdog），欢迎分享，非公众号转载请保留此声明。

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

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

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

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

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

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

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18

ExecutorService service = Executors.newFixedThreadPool(10);
...
service.submit(()->{
    while(true){
        if(CollectionUtils.isEmpty(items)){
            break;
        }
        List<Data> items = queryPageData(start, end); // 分页逻辑
        for(Data item : items){
            try {
                Thread.sleep(10L);
            } catch (InterruptedException e) {
                //noop 
            }
            processItem(item);
        }
    }
});

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

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

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

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

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

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

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

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

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

且看修改后的代码。

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24

ExecutorService service = Executors.newFixedThreadPool(10);
...
service.submit(()->{
    while(true){
        if(CollectionUtils.isEmpty(items)){
            break;
        }
        List<Data> items = queryPageData(start, end); // 分页逻辑
        for(Data item : items){
            try {
                Thread.sleep(10L);
            } catch (InterruptedException e) {
                //noop 
            }
            try{
                processItem(item);
            }catch(Exception ex){
                LOG.error(...,ex);
            }
        }
    }
});
...
service.shutdownNow();

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

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

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

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

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

1
2
3
4
5

try {
    Thread.sleep(10L);
} catch (InterruptedException e) {
    break;
}

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

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11

while (true) {
    Thread currentThread = Thread.currentThread();
    if(currentThread.isInterrupted()){
        break;
    }
    try {
        Thread.sleep(1L);
    } catch (InterruptedException e) {
        currentThread.interrupt();
    }
}

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

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

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

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

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

1
2
3
4
5

try{
    processItem(item);
}catch(Exception ex){
    LOG.error(...,ex);
}

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

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

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

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

果然是不同凡响的发问。

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

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

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

大家不欢而散。

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

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

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

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

作者简介：小姐姐味道 (xjjdog)，一个不允许程序员走弯路的公众号。聚焦基础架构和 Linux。十年架构，日百亿流量，与你探讨高并发世界，给你不一样的味道。

推荐阅读：

1. 玩转 Linux
2. 什么味道专辑
3. 蓝牙如梦
4. 杀机！
5. 失联的架构师，只留下一段脚本
6. 架构师写的 BUG，非比寻常
7. 有些程序员，本质是一群羊！

小姐姐味道 不羡鸳鸯不羡仙，一行代码调半天 329 篇原创内容 –>