5

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;
}


  • If you want to keep passing the exception up, then catching and re-throwing is fine. If the code is catching it, logging it, and then throwing it again, that seems odd. - Andrew

5 답변


2

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.


  • I don't think it is logging anything anywhere. It is just making sure that the exception caught is displayed and asking client to handle it appropriately the way it wants to by taking measure like you suggested logging exception at a central place. - Prateek
  • @Prateek: by logging i meant writing to stdout (very crude, but it's a kind of logging). re-worded just now. - Nathan Hughes
  • Ok, but even with that rethrowing an exception might actually be good because different clients might try to handle these exceptions differently. Don't you agree? - Prateek
  • @Prateek: Throwing a new exception with the old one nested inside would be my preferred way. that way the new exception can add more information, and the exception type can be different. - Nathan Hughes
  • Yeah that is what I would go after but this one is not that bad either :) - Prateek

1

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:

  • It catches runtime exception too.
  • You lose information about the type of exception that was thrown.
  • You cannot react to or handle specific exceptions.

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.


1

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.


  • Are you sure about your claim that you have to throw a new exception in Java? - pamphlet
  • @pamphlet The point is when you have to throw an customized exception you have to create an object of exception class(or of one extending it) and then you can throw it. In the above code there is no exception object create other then e - Prateek

0

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.


  • Another option would be to use an appropriate logging mechanism. - pamphlet

0

It can be a good practice. I think I always use conversion to RuntimeExceptions 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 RuntimeExceptions as they require less typing.

Catching Exception

  • Checked exceptions (IO exceptions, security errors, concurrency, etc)
  • Runtime exceptions (anything, unpredicted garbage, see below)
  • Everything - these are 99% of all errors (there are Errors left however)

Catching RuntimeException

  • null pointer exceptions, index out of bounds exceptions, access exceptions, + API which wraps propagates exceptions into RuntimeException - this is ALSO A LOT

My 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.


  • Throwing a general 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 Paliath
  • Yeah, that's correct. What I mean that it makes no diffence for the calling party. Catching general Exception means 'catching everything'. Catching RuntimeException means 'catching anything'. I haven't seen real-world difference between two cases yet. - Andrey Chaschev
  • It will make a difference to the calling party. If the method is throwing Exception, 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 Paliath
  • Yep, there is logical difference. I haven't found a use of it yet. In most cases throwing a general Exception 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

Linked


Related

Latest