Java exception generalization: bad practice?

Question

Coming from the world of PHP, where there is only one way to write exception handling .. I found that the exception wrapper in Java is a bit ugly:

public void exampleOneException(String input) throws MyBusinessException { try { // do something } catch (NumberFormatException e) { throw new MyBusinessException("Error...", e); } } 

Instead, I prefer to use this style:

 public void exampleTwoException() { try { // do something } catch (MyBusinessException e) { log.error("Error...: " + e); } catch (NumberFormatException e) { log.error("Error...: " + e); } } 

Is there any difference or best practice regarding these exception handling approaches?

Answer 1

These are both valid approaches for two different scenarios.

In the first scenario, the method cannot do anything reasonable with respect to the exception, but it must "communicate" it up. Thus, the caller can catch the exception and decide what to do with it (for example, cancel the stream, display the message to the user, record it, etc.)

In the second scenario, you detect an exception and register it, effectively hiding the rejection of the caller. This type of processing can be useful if the caller does not care if the operation was successful or not.

Answer 2

The first example is usually seen as the best approach.

You should also not consider a MyBusinessException to wrap a NumberFormatException , but most likely a NumberFormatException is the cause of a MyBusinessException .

Exceptions must be appropriate for the interface. The subscriber of your interface does not need to know or describe the implementation details. If a NumberFormatException really makes sense as a type of error when calling exampleOneException , it should be thrown into a more appropriate exception.

A more specific example, as a rule, includes various implementations in which interface users are not required to be required for implementation specifics (which may not even be known at compile time).

 interface MyRepository { Object read(int id) throws ObjectNotFoundException; } // a sql backed repository class JdbcRepository implements MyRepository { public Object read(int id) throws ObjectNotFoundException { try { ... } catch (SQLException ex) { throw new ObjectNotFoundException(ex); } } } // a file backed repository class FileRepository implements MyRepository { public Object read(int id) throws ObjectNotFoundException { try { ... } catch (FileNotFoundException ex) { throw new ObjectNotFoundException(ex) } } } 

Because an interface declares the types of errors that it can return, clients of that interface can be consistent and rational. By adding code to handle FileNotFoundException and SQLException , then the real implementation may be either or not fantastic.

Consider whether there were several places in the FileRepository implementation that could throw a FileNotFoundException . Does this mean that each of them implies that the object was not found?

When considering the second variant of exampleTwoException it is important to understand that your catch effectively states that the consequences of any error have been mitigated. There are times when checked exceptions are reasonably ignored, but it is much more likely that simple

 try { ... } catch (SomeException ex) { log.error("caught some exception", ex); } 

it is actually the result of the developer not taking into account the consequences of the exception, or the code must include FIXME .

This is even more true if you see catch (Exception ex) or incompatible catch (Throwable ex) .

And finally, do you want to be the person who could dig through the application, finding all the places where you need to add another catch block to handle the new implementation? Perhaps the reason is catch (Exception ex) ...

Always throw exceptions corresponding to abstractions

Answer 3

I would say that both NumberFormatException and MyBusinessException are useful, but in different cases.

Usually they are displayed at different levels of the class hierarchy: for example, NumberFormatException is a lower-level exception, and you cannot expose it to a higher level (for example, the user interface) if the user of this user does not have the right to recover from it. In this case, it is more elegant to simply throw a MyBusinessException and display an informative message explaining, for example, that something in the previous step was badly delivered or some internal processing error occurred and he / she needs to restart the process.

On the other hand, if your function is used at an intermediate level (for example, API), and the developer has means to recover from exceptional behavior, NumberFormatException more useful, since it can be viewed programmatically and the application flow can continue with minimal interruption (for example, indicate a valid default number). Alternatively, this may indicate a flaw / error in the code that needs to be fixed.

For more information on how to follow best practices for using exceptions, read Item 61 - Throw exceptions appropriate to the abstraction of Effective Java by Joshua Bloch .

Answer 4

In terms of clean and solid code, you are not moving in the right direction.
First, you do not need to use try / catch for the Exception that is already set by the method header. Therefore, in your first case, the code should look like this:

  public void exampleOneException(String input) throws MyBusinessException { // do something } 

It's prettier, isn't it? In the above case, if a MyBusinessException , the exception will be swallowed, so the user will never see what happened. If you want this to happen, this is your best practice.

The second case is clear. You pick two different exceptions and you process them (by registering), which is best practice in general.

So the best practice of your two different cases depends on what you want to achieve. As for beauty, this is completely subjective.