This question already has an answer here:
In some legacy code I see this, that an overbroad exception is being caught, and then thrown again, Is this a good practice? Does throw e;
rethrow the same exception, or create a new one ?
catch (Exception e) {
StringBuilder sb = new StringBuilder(
"Oops. Something went wrong with id: ");
sb.append(id);
sb.append(". Exception is: ");
sb.append(e.toString());
System.out.println(sb.toString());
throw e;
}
throw e
is rethrowing the same exception. At least it preserves the original stacktrace. It's just writing a message to stdout recording some information about what happened, then letting the original exception proceed on its way.
It's not a great practice, it should be enough to log the exceptions in a central place, recording their stacktraces. If you need to add more information (as in the example, where it logs an id), it's better to nest the original exception in a new exception as the cause, then throw the new exception. I would guess this probably occurs in contexts where there is no centralized logging or where exceptions tend to get eaten somewhere.
rethrowing
an exception might actually be good because different clients might try to handle these exceptions differently. Don't you agree? - Prateek
This is usually a bad practice. catch(Exception e)
(sometimes called Pokemon Exception Handling for when you gotta catch 'em all) catches every single exception. Such exception handling is rarely useful because:
Your method signature now is public void whatever() throws Exception
, which is rarely useful. Now everything further up the chain has no idea what kind of exception you have thrown; they will have to do instanceof
checks which defeats the purpose of catching specific-exceptions entirely.
As far as your second exception is concerned, throw e;
throws the same exception object. If you wanted to wrap the exception, you can create a new one, which means you would do something like throw new MyCustomException(e);
. You would also need to change your method signature.
If there is nothing further up the chain, I guess this isn't as bad (still isn't great, though). It looks like a method that is trying to log all exceptions that are thrown. However, again, there are better ways of doing this.
My best guess would be it is trying to have two layers of protection. Making sure that an error message is displayed and also asking the client to handle the exception the way it wants to because the catch
clause is not doing anything to recover from the exception.
I won't consider it good/bad practice. Depending on your requirements you can consider to go either way like there might be 100 of different clients using your API and each one of them has a different way of recovering from an exception
. By displaying what went wrong it is adding a default action layer just below the layer where client decides on how it will handle the exception
.
Now back to your question. I think throw e
throws the same exception object. As exceptions are objects
in java you need to create a new exception object
before you can throw
it which I can't see happening in your code.
throw
it. In the above code there is no exception object create other then e
- Prateek
throw e
does throw the same exception. There might have been reasons for doing this, but there is also a reason not to. In your example code, a message is sent to System.out
, but if a stack trace were printed later on System.err
, it won't be syncronized, and in fact the two might end up interwoven in your console.
A better approach would be the following:
catch (Exception e) {
StringBuilder sb = new StringBuilder(
"Oops. Something went wrong with id: ");
sb.append(id);
sb.append(". Exception is: ");
sb.append(e.toString());
throw new Exception(sb.toString(), e); // The original exception is an argument
}
This will create a new Exception
with the modified message, and append the original Exception
to the stack trace to help with debugging.
It can be a good practice. I think I always use conversion to RuntimeException
s during prototyping. After this if there is a need one can change this into better exception handling. For this my purpose there is an utility class in Guava called Throwables which makes exception propagation.
In your case however the exception should be converted into a runtime exception, because declaring a method throwing a general Exception
is just the same as a method throwing RuntimeException
for the calling party. In the first case it 'catches everything', in the latter it 'catches anything'. I have not yet experienced the difference between two of these in real-world applications. So I prefer RuntimeException
s as they require less typing.
Catching Exception
Error
s left however)Catching RuntimeException
RuntimeException
- this is ALSO A LOTMy point is after when you're catching an Exception
you can't really handle all these cases. So it makes no difference except for the less typing for the calling party if you wrap it into a RuntimeException
.
Exception
is not the same as throwing RuntimeException
. The former is checked and so will need to be mentioned in the method signature. Any exception that is a subclass of Exception
but not a subclass of RuntimeException
is a checked exception. - Vivin PaliathException
means 'catching everything'. Catching RuntimeException
means 'catching anything'. I haven't seen real-world difference between two cases yet. - Andrey ChaschevException
, then the caller has to either handle it or rethrow, whereas the caller doesn't have to do either if the method is throwing RuntimeException
. Also catching RuntimeException
doesn't mean "catching anything". It only catches those exceptions that are subclasses of RuntimeException
(and RuntimeException
itself); it won't catch any checked exceptions. - Vivin PaliathException
is a bad practice in Java, so it would be really hard to think of a real-world example. Propagating a RuntimeException
can not however be an anti-pattern and it can be a good practice. - Andrey Chaschev