代码Review,瑞出事来了!

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

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

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

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

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

一个任务处理例子

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

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

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);
}
}
});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将统一处理这些异常。

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

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

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

不要生吞异常

且看修改后的代码。

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的时候,终止循环。

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

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

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粒度是不对的,太粗鲁。

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

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

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

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

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

果然是不同凡响的发问。

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

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

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

大家不欢而散。

End

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

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

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

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

责任编辑:赵宁宁 来源: 小姐姐味道
相关推荐

2021-03-03 07:28:58

ReviewAuthor代码

2013-10-24 09:43:58

代码代码审查

2012-07-05 09:45:02

代码审查

2009-08-05 09:59:40

Code Review代码审查工具

2012-09-03 13:41:50

Code Review

2022-06-23 09:57:01

code-revie前端代码

2020-09-22 08:06:45

代码事故

2021-06-07 16:01:15

代码开发工具

2021-05-13 11:35:54

K8STerraform代码仓库

2020-03-06 10:14:18

IT支出IT投资

2018-08-16 15:11:47

Code ReviewPPT代码

2020-11-02 09:03:22

代码开发工具

2022-10-27 10:33:48

敏捷开发开发

2015-11-17 16:11:07

Code Review

2023-11-22 11:22:57

AI模型

2020-02-10 20:16:04

程序员AI人工智能

2023-05-31 14:49:07

Firefox火狐浏览器

2024-11-08 14:18:38

2011-05-16 14:12:30

QuickWidgetQML
点赞
收藏

51CTO技术栈公众号