Java exceptions wrapping: bad practice?

Question

Coming from a PHP world where there's only one way to write exceptions handling.. I found the wrapping of exceptions in Java a bit "ugly":

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

I prefer to use this style instead:

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 practices on those approaches on handling exceptions?

Autosuggestion

Methods that call exampleOneException can catch MyBusinessException and handle it according to their own requirements. Methods calling exampleTwoException have no way of knowing that anything went wrong. – Transformism

These are doing fundamentally different things. The approach you should use depends on how you want to respond to errors - e.g. just logging them locally, forwarding them on, or dedicated business logic that changes behaviour when an error occurs. – Karlow

Those two code snippets aren't doing the same thing. In one you are rethrowing the exception in the other you are just logging it. – Fertile

I believe exception handling in quite useless. I mean if you're doing some operation that you know it might go wrong but have an alternative solution, why are you even letting it go wrong? Either you don't have a clue of the correct logic to prevent the exception or you don't really have an alternative strategy and just hope everything works fine. Either scenario is bad programming and is confusing to read. If something has two valid alternate outcomes then just do a check before the dangerous operation is invoked. – Gabi

Answer 1

These are both valid approaches for two different scenarios.

In the first scenario, the method can't do anything intelligent about the exception, but it has to "report" it upwards. That way the caller can catch the exception and decide what to do with it (e.g. cancel the flow, pop a message up to the user, log it, etc.)

In the second scenario, you catch the exception and log it, effectively hiding the failure from the caller. This type of handling may be useful if the caller doesn't really care if the operation succeeded or not.

Patten

You could also write nothing into to catch clause if you are certain that the operation will be successfull. It´s regarded as bad programming but nevertheless widely used simply because people are lazy^^ – Stenotypy

Answer 2

The first example would generally be seen as the better approach.

You should also not consider the MyBusinessException to be wrapping the NumberFormatException, but rather the NumberFormatException is the cause of the MyBusinessException.

Exceptions should be appropriate for the interface being exposed. A caller of your interface should not need to know, or deal with, the implementation details. Unless a NumberFormatException genuinely makes sense as a type of error when calling exampleOneException, it should be translated into a more suitable exception.

A more concrete example typically includes differing implementations, where the users of the interface should not be required to deal with the 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 the interface declares the types of errors it can return, the clients of that interface can be consistent and rational. Adding code to handle FileNotFoundException and SQLException then the actual implementation may be either, or neither, is not fantastic.

Consider if there were multiple places in the implementation of FileRepository which could throw FileNotFoundException. Does that imply each and every one of them implies object not found?

When considering option two, exampleTwoException, it's important to realise that your catch block is effectively stating that the effects of whatever error occurred have been mitigated. There are cases where checked exceptions are reasonably ignored, but it is far more likely that a simple

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

is actually the result of a developer not considering the consequences of the exception, or the code should have included a FIXME.

This is even more true when you see catch (Exception ex), or the unconscionable catch (Throwable ex).

And finally, would you want to be the person to dig through an application finding all the places where you need to add (yet) another catch block to handle a new implementation? Perhaps that's a cause of catch (Exception ex)...

Always throw exceptions appropriate to the abstraction

Fabria

Answer 3

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

They usually appear at different levels of class hierarchy: for example NumberFormatException is a lower-level exception and you might not want to expose it at a higher level (e.g. user interface) if the user of it has no power to recover from it. In this case it is more elegant to just throw MyBusinessException and display an informative message that explains for example that something in a previous step was badly supplied 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 (e.g. API) and the developer has the means to recover from the exceptional behavior, NumberFormatException is more useful, as it can be dealt with programmatically and the flow of the application might continue with minimal interruption (e.g. supply a default valid number). Alternatively, this can indicate a flaw/bug in the code that should be fixed.

For details about how to follow best practice in using exceptions, read Item 61 - Throw exceptions appropriate to the abstraction from Effective Java by Joshua Bloch.

Pictograph

Answer 4

From the perspective of clean and solid code you are not driving to the right direction.
Firstly you do not need to use try/catch for an Exception that is already thrown by the method's header. So in your first case the code should be like below:

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

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

The second case is clear. You catch two different exceptions and you handle them (by logging) which is a better practice generally speaking.

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

Amelioration

try/catch is needed when I need to use finally: like if I want to close a DB connection instead of relying on the Java garbage collector. – Autosuggestion

Simply explanation: try/catch is needed in case you want to handle an Exception. If you do not care for propagation or handling then you can use throws on method's header. Using finally presupposes a handling intention. – Amelioration

well if we care about code quality and therefore use a linting library (sonarlint) then using finally is a "good practice" or we will get the linter show an error: "Resources should be closed".. but that's another topic.. – Autosuggestion

You are right, using finally is actually a good practice. Although my answer pointed to an other issue. – Amelioration