Original: Little Sister Taste (wechat official ID: Xjjdog), welcome to share, please keep this statement if reprinted by non-official account.
Not long ago, the department conducted a code review.
The code as a whole is relatively simple, the B has been blown, nothing more than the old if and else problems. When I turned to the one-step execution code for a scheduled task, my eyes lit up and I thought it was time for a BB.
To my surprise, these guys were full of recognition during the judging, but soon after the judging was over, they gave me a “B”.
Today I put at that time of these words to sort out, let everyone say, I in the end is not a B. Dubious!
A task processing example
The structure of the code looks like this.
With timing, this code checks the records in the database every night in the early hours of the morning. The main logic is to use separate threads to progressively read the relevant records in the database, and then put those records in a loop and process them item by item.
ExecutorService service = Executors.newFixedThreadPool(10); . service.submit(()->{ while(true){ if(CollectionUtils.isEmpty(items)){ break; } List<Data> items = queryPageData(start, end); // Paging logic for(Data item: items){try {thread.sleep (10L); } catch (InterruptedException e) { //noop } processItem(item); }}});Copy the codeWait a minute. I call a halt when the code is flipped over, and the processItem here does not catch an exception.
Normally, this wouldn’t be a problem. But the quiet years are punctuated by random accidents. If this is the complete code for your task, it has a very subtle way of troubleshooting. Even if your unit tests are well written, this code can still be poisoned remotely by logging problems.
Yes. The root cause of the above code is not catching exceptions that the processItem function might raise. If any exception, whether checked or unchecked, is thrown during record processing, the entire task is terminated!
Don’t think it’s easy. If you step on this pit, please remember to buckle 666. Or take a look at your task execution code to see if you have the same problem.
The Java compiler will tell you that it caught an exception in many cases, but there are always exceptions that get away, such as null-pointer exceptions. As shown below, RuntimeException and Error are both examples of unchecked exceptions.
RuntimeException can be used without a try… Catch, but if an exception occurs, it will cause the program to abort, and the JVM will handle the exception uniformly.
If you don’t catch it, it will be the end of your mission.
If you want to perform tasks asynchronously, it’s best to put a little more effort into exception design. There’s a lot of rollovers here, and this car doesn’t mind taking you with it.
The judge was very modest and immediately modified the code on the spot.
Don’t eat anything raw
Take a look at the modified code.
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();
Copy the codeIn order to control the frequency of task execution, sleep is an effective method.
The code is thoughtful enough to catch exceptions the way we did above. He was also thoughtful enough to capture the anomalies associated with sleep. This is not sweet and there is no way, because if you do not complete this part of the code, the compilation will not pass, let’s assume that the level of developers is good enough.
Because Sleep throws InterruptedException, the code does nothing. This is also a common operation in our code. If you don’t open your project, there is a ton of code that ignores InterruptedException.
At this point, you execute the code, and even though the thread pool is using the violent shutdownNow function, your code still can’t terminate, it just keeps running. Because you ignored InterruptedException.
Of course, we can terminate the loop if we catch InterruptedException.
try {
Thread.sleep(10L);
} catch (InterruptedException e) {
break;
}
Copy the codeWhile this does what is expected, InterruptedException is generally not handled this way. The correct way to handle it is this:
while (true) { Thread currentThread = Thread.currentThread(); if(currentThread.isInterrupted()){ break; } try { Thread.sleep(1L); } catch (InterruptedException e) { currentThread.interrupt(); }}Copy the codeIn addition to capturing it, we have to reset the interrupt state again, otherwise it will be captured and cleared. InterruptedException is important in many scenarios. InterruptedException is useful when methods that block a thread, such as time-consuming calculations, leave the entire thread stuck and unable to do anything.
But it doesn’t make any difference to the logic of our current code. The little partner who was evaluated said unsatisfactorily.
Another question!
Impact is one thing, good habit is another. I did my best to install B, but there are other hidden problems in your exception handling code.
Any other questions? We’ve all changed our lazy faces. Tell me about it.
Let’s take a look at the question of the friend’s on-site change. He uses catch directly to catch the exception here and logs it accordingly. The problem I have to say is that the Exception granularity here is wrong and rude.
try{ processItem(item); }catch(Exception ex){ LOG.error(... ,ex); }Copy the codeThe processItem function throws IOException as well as InterruptedException, but we all treat it as a plain Exception so that it does not show the intent of the upper function.
For example, the processItem function throws a TimeoutExcepiton, expecting us to do some retries based on it; Or throw SystemBusyExcption expecting us to sleep a little longer to give the server a little more time. Such coarse-grained exceptions run the risk of catching them all at once, leaving the code undetected when new exceptions are added.
For a moment the room was silent.
I think what you said is quite right, said a relatively senior veteran bird. You mean to catch all abnormal cases separately and carry out fine processing. But you still end up using Exception to catch RuntimeException, and you still can’t catch exceptions.
It was a remarkable question.
One of the things that makes good, standard code writing unenforceable is that the rest of the code in the project simply doesn’t follow the rules. If our underlying code had done the right null-pointer judgment, array out of bounds, or pre-exception translation using an API like Guava’s Preconditions, the above question wouldn’t have to be answered at all.
In the case of the above code, however, we need to manually catch RuntimeException for separate processing.
Your project has too much bad code to change. I have emotional intelligence, but I have more temper.
They parted in bad humour.
End
I don’t get it. Code review is used to find problems. As a result, the review meeting was held and everyone was laughing at me behind my back. Is this really my problem? Is it the team? It’s confusing.
I didn’t say anything when you were debating whether to use an Integer or an int, but now I’m going to talk about exception handling. You can’t have all the B’s.
What? You want to review my code? Do I code like I say I do, do I lead by example? Sorry, I’m an architect, and I haven’t written code in years.
This wish of yours has failed you!
Xjjdog is a public account that doesn’t allow programmers to get sidetracked. Focus on infrastructure and Linux. Ten years architecture, ten billion daily flow, and you discuss the world of high concurrency, give you a different taste. My personal wechat xjjdog0, welcome to add friends, further communication.
Recommended reading:
2. What does the album taste like
3. Bluetooth is like a dream 4. 5. Lost architect, leaving only a script 6. Bugs written by architect, unusual 7. Some programmers, nature is a flock of sheep!
Taste of little sister
Not envy mandarin duck not envy fairy, a line of code half a day
329 original content