7

I'm trying to understand the right way of using java catch blocks. Should I write business logic there or just suppress an error?

My question in some part related to this one. Check it out, please.

What I understand:

If it unchecked exception the best way is to write code in next way:

Integer n = null;
try {
    n = Integer.parseInt(reader.readLine());
}
catch(NumberFormatException e){
    log.error('Can't parse string');
}
if (n == null) {
    n = 0;
}

and avoid code like this one:

Integer n = null;
try {
    n = Integer.parseInt(reader.readLine());
}
catch(NumberFormatException ignored){
    n = 0;
}

My question:

But what it will be checked exception, and I work e.g. with a database wich throws exception NoSuchElement, when searching element was not found:

User user = null;

try {
    user = User.findById(userId);
} catch (NoSuchCategoryException e) {
    log.error('User {} doesn't exist.', userId);
    user = new User();
}
user.setUsername('someName');
etc...

So can I write code like listed above or should use the same pattern like in first my example?

Cœur
  • 37,241
  • 25
  • 195
  • 267
Vladyslav Sheruda
  • 1,856
  • 18
  • 26
  • There's no difference between the "preferred" and "avoid" approaches. In both cases, if the exception is thrown, `n` will still be assigned to 0; only in the second one is an unnecessary `null` check removed (which is to say I *might* prefer the second approach, but it really depends on what you're doing). – Makoto Nov 30 '15 at 08:06
  • 1
    *Don't* "just suppress an error": handle it appropriately. "Appropriate handling" is hard if you are catching `Exception`, because it could be literally any type of exception, in the same way as a method returning `Object` could return anything. Catch the most specific type of exception you know how to handle (e.g. `NumberFormatException`). – Andy Turner Nov 30 '15 at 08:06
  • 2
    This is a classical example of "primarily opinion-based" question. – Andremoniy Nov 30 '15 at 08:08
  • To narrow it down a bit, there are other questions to ask: why would the database throw a *checked* exception, since checked exceptions presume that you can recover from them somehow? – Makoto Nov 30 '15 at 08:09
  • sorry, my fault, edited. But this is not the answer. – Vladyslav Sheruda Nov 30 '15 at 08:09
  • @Makoto liferay throws exception like this, and this is checked exception, – Vladyslav Sheruda Nov 30 '15 at 08:10
  • Please, if you don't have thoughts about my question, then don't ask question which are not related to listed above. – Vladyslav Sheruda Nov 30 '15 at 08:12
  • While the question that you've posed in its current form is too broad, you can do a bit of digging on this as well. Since the implication is that you're somehow able to recover from this exception, what is the best way according to your business logic to recover? This isn't something that we can answer without being hired as consultants and signing many NDAs first, but this gives you some time to gnaw over it. – Makoto Nov 30 '15 at 08:12
  • @Makoto I think that in java should be some convention or pattern, which will throw a light on my question. – Vladyslav Sheruda Nov 30 '15 at 08:14

2 Answers2

2

Catch blocks are regular control flow, there is no point in avoiding them.

However, don't catch the error in places that can't handle it yet. You may want to know that there was an input error in a place further away.

Manually signalling errors by replacing the result with null is very error prone (NullPointerException hooray) and you easily lose the meaning of the null. Manually adding all the control flow to propagate such a replacement value on a short path back to the caller is also not always great because you would get all this for free if you used the exception. The rule of thumb is: less code is cleaner.

The golden rule for exceptions is "catch late" (or more general "handle late"): In the place that contains the business logic that knows what to do. In some cases that means replacing a checked exception with an unchecked one so it can propagate through predefined interfaces that don't allow adding throws clauses.

Integer n = null;
try {
    n = Integer.parseInt(reader.readLine());
}
catch(NumberFormatException e){
    log.error('Can't parse string');
}
if (n == null) {
    n = 0;
}

This is the most problematic version. It's superficially avoiding the catch block and adds unneeded code & code branches. You should use what you have in the second example because it's way cleaner - That is, in case your business logic actually is to replace input errors with 0 and silently ignore them otherwise. It's most of the time not. Immediately returning control & information what happened back to the caller often is.

Not catching here might be right(er). Or catching and replacing the exception with regular API or a different Exception. What's right for you depends.

What the other question tells you is not to overuse exceptions in your own APIs. You can find a clean API / abstraction that signals the error without using exceptions. Not using Exceptions is especially useful when the place throwing the exception and the one that needs to handle them is conceptually far away (architecture layers etc). Exceptions as API aren't great in those cases because it leads to either a bunch of throws clauses that everybody has to have (which is adding dependencies on details https://en.wikipedia.org/wiki/Dependency_inversion_principle ) or code that needs to have knowledge of implementation details of other code, i.e. which unchecked exceptions are thrown somewhere else.

But if you contain exceptions within a reasonable perimeter they are absolutely fine to use and can make life easier. It's not against the rules to use all the capabilities of plain old java.

User user = null;
try {
    user = User.findById(userId);
} catch (NoSuchCategoryException e) {
    log.error('User {} doesn't exist.', userId);
    user = new User();
}

looks like a typical case where you shouldn't handle the error like that. "Fake" database objects can easily lead to code that assumes there was a proper entry. Assuming that's the case and you want to avoid exceptions you could do roughly

try {
    return Optional.of(User.findById(userId));
} catch (NoSuchCategoryException e) {
    log.error('User {} doesn't exist.', userId);
    return Optional.empty();
}

and handle the case with a clean abstraction. Or you return in case of exception but continue in case everything is ok. But continuing with a fake object is often not right. It can be.

Gandhi
  • 11,875
  • 4
  • 39
  • 63
zapl
  • 63,179
  • 10
  • 123
  • 154
  • e.g. I'll convert my NoSuchUserException in unchecked. What should I do then with unchecked exception? – Vladyslav Sheruda Nov 30 '15 at 08:31
  • @JamesSchermann catch them where you can decide what to do with them. Some place has to implement the "I'll try to do ... if that fails then ..." logic. The code that catches them should probably be reasonably close so it can know that the code it's using uses such exceptions. – zapl Nov 30 '15 at 08:59
  • Hm, ok, thanks a lot for your answer. – Vladyslav Sheruda Nov 30 '15 at 09:03
  • @JamesSchermann It's complicated :) Also updated the answer a little – zapl Nov 30 '15 at 09:49
-1

You can use any code you want on any catch block. For example, I usually use those to break loops or show special events on the User Interface.

Mayuso
  • 1,291
  • 4
  • 19
  • 41