Search code examples
phpexceptionconceptual

Is it a bad practice to never re-throw exceptions in a program?

I've just read some articles about exceptions and the most part of the examples do this kind of stuff:

try{
    conn.close();
} catch(SQLException ex){
    logger.error("Cannot close connection");
    throw new RuntimeException(ex);
}

While I'm used to do this:

    try
    {
        $this->pdo = new PDO($dbInfo[0], $dbInfo[1], $dbInfo[2]);
        $this->pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    }
    catch(PDOException $e)
    {
        $this->exceptionHandler->displayException($e);
    }

As you can see, I use a class to handle exception instead of re-throwing it.

For instance, if a class A uses a instance B which uses a instance C which can throws an exception, I will directly handle it in the class B and not re-trowing it to the class A.

However, according to the first example, it seems that the guy just re-throws all the sub classes exceptions to a main class (like A by using my example).

Is my method bad?

Solution

  • In your displayException() example, you're not doing anything else beside displaying the exception (assuming the method does what the name implies). This might be fine if you don't have any additional code running that relies on DB activity - which might be the case for a simple PHP app/script but it's hardly ever the case for Java applications. In a more complex application, you usually want to abort the execution of any further logic and dealing with the exception on a higher level, thus you'll want to be re-throwing the exception (in Java, often as an unchecked exception instead of checked one - PHP only has the unchecked ones).

    About the choice in Java to convert a checked exception to an unchecked one, I heavily recommend article Effective Java Exceptions. It explains Java exception model in depth and when you should be doing what. It is a matter of opinion I guess, but I think the arguments laid out there are solid.

    In the specific case SQLException you gave, there is somewhat a consensus in java community that it a) shouldn't have been a checked exception to begin with, and b) is too general to convey anything meaningful about the actual error. This is why you often either

    • catch it, add any relevant details to the exception or the logs and re-throw it as an unchecked exception or
    • use an exception translator like the one in Spring that does this for you automatically, or in the case of exception in a finally block,
    • use an auxiliary method that closes it quietly such as closeQuietly