0

Consider following example. Is it considered a bad practise?

Note: I know re-throwing exception is ok, but what about assertionerror?

public static main(){
    try {
        doSmth();
    } catch (WhateverException we) {
        throw new AssertionError(e.getMessage());
    }
}

public static void doSmth() throws WhateverException { }
Peter
  • 340
  • 4
  • 13
  • 7
    1) It doesn't compile. 2) It swallows the stacktrace. 3) it can throw an NPE at exception creation (if it were changed to compile). 4) you should only ever throw classes that inherit from `Exception`, `AssertionError` is an `Error` not an `Exception`. In short, almost everything about this code is bad practice. – Boris the Spider Jun 11 '18 at 06:32
  • Well `AssertionError` is quite specific. If you're not doing comparisons and checks i would not recommend to throw an `AE`. Also throwing errors is mostly bad practise, unless you really want to kill your program – Lino Jun 11 '18 at 06:32
  • Why would you want to do that anyways? ...Assert is meant for Unit Testing – Rcordoval Jun 11 '18 at 06:34
  • I don't think there is a point in throwing an Exception from inside another Exception. Throwing an Error is even worse. – Diego Victor de Jesus Jun 11 '18 at 06:34
  • 3
    @Rcordoval whilst it is true that assertions are _used_ in unit tests, an `AssertionError` is thrown by the Java [`assert` keyword](https://stackoverflow.com/questions/18907487/correct-use-java-assert-keyword) and this features in production code. – Boris the Spider Jun 11 '18 at 06:38
  • 2
    @DiegoVictordeJesus Exception chaining is very common and definitely has a point. See https://stackoverflow.com/questions/5020876/what-is-the-advantage-of-chained-exceptions. – Sneftel Jun 11 '18 at 06:39
  • 2
    @DiegoVictordeJesus sorry, but you think wrong. The whole purpose of exceptions is to bubble up the stack. The user clicks a save button, that action goes all the way to the IO subsystem and that throws an `IOException` - as this exception travels up the stack each frame can add more context by wrapping and rethrowing (which file? what was the format? where in the UI?) etc - the final exception the UI gets will then have enough information to produce a meaningful error to the user - which just propogating an `IOException` would not. – Boris the Spider Jun 11 '18 at 06:45
  • Now I can see the point. Does this applies to runtime exceptions as well? – Diego Victor de Jesus Jun 11 '18 at 07:31
  • @DiegoVictordeJesus this applies to all exceptions - there is an open debate as to whether non-runtime exceptions should ever be used at all. – Boris the Spider Jun 11 '18 at 07:38

1 Answers1

2

It's not bad practice to throw an error in response to an exception, if the exception indicates a situation that is fatal for your code. However:

  • There's no need to use AssertionError in particular. I mean, it's fine if you do, since nobody's going to be catching one, but you should consider just doing Error instead, so that somebody doesn't go looking for the assert statement that they assumed caused it.
  • When chaining a throwable like that, always use the old exception to construct the new one: throw new AssertionError(we). That'll keep the old stack trace around. You can (and should) also pass a custom message.
Sneftel
  • 40,271
  • 12
  • 71
  • 104
  • 2
    Not sure I agree - I would state that throwing an `Error` is always wrong; the JVM is allowed to throw errors, you are not. Thowing a well named `RuntimeException` is the correct approach. – Boris the Spider Jun 11 '18 at 06:40