5 Pitfalls Involving Java Exceptions

After looking at a fair number of application source codes written by others, i have noticed that these are some of the most common pitfalls involving exceptions. i must admit that i have committed them at one time or another as well -- earlier, due to ignorance, and later on, due to convenience (but only in trivial, run-once-and-throw utilities). i am sure there are other common mistakes and bad habits in this area, but these few i have come across most often.

1. Catching an exception, then doing absolutely nothing.

try
{
    FileReader in = new FileReader("MyFile.txt");
}
catch (FileNotFoundException ex) {}

This, in my humble opinion, is the single worst thing to do with an exception. Should the exception occur, the offending application continues to run in an indeterminate state. At best, another exception (usually a NullPointerException) will terminate the application a few lines later. At worst, it just continues to inexplicably (to the programmer) churn out erroneous results.

There is a, well, exception to this though. When we clean up resources in finally blocks (e.g. closing connections, streams, etc.), We typically enclose those statements in try-catch blocks to allow the finally block to run to completion.

InputStream in = null;

try
{
    // Do Stuffs Here
}
finally
{
    if (in != null)
    {
        try
        {
            in.close();
        }
        catch (IOException ignore)
        {
            // Ignore
        }
    }
}

http://www.digg.com/programming/5_Pitfalls_Involving_Java_Exceptions

In such cases, it is "forgiveable" to ignore the exception. The failure to close a resource usually does not prevent an application from continuing to run correctly, and there is also little corrective action that can be done about it. However, it is still better to log the exception (perhaps with a lower priority), so that if indeed a resource leakage is detected later on (and resource leakages are usually not obvious right away), the cause of the leakage can be traced.

2. Allowing an exception to manifest itself to the user, and terminating the application.

For desktop applications, this means throwing exceptions out of the main method.

public static void main (String[] args)
        throws IOException, FileNotFoundException
{
    // Do Stuffs
}

This is bad because usually, the user cannot do anything about the exception that just showed up, and in most cases, the stack trace would not even make any sense to him. In the worst case, the user could be halfway through a critical piece of work when the application crashed on him.

3. Not ensuring that the finally block completes normally.

As mentioned above, an important practice is to make use of finally blocks to clean up resources before they are garbage-collected, and these cleaning-up statements are wrapped in try-catch blocks, but sometimes, it is not done correctly.

FileInputStream in = null;
OutputStream out = null;

try
{
    // Do Stuffs
}
finally
{
    try
    {
        in.close();
        out.close();
    }
    catch (IOException ignore)
    {
        // Log Exception
    }
}

The problem here is that, if the in.close() statement throws an exception, the out.close() statement will not even be attempted, and the output stream will be left open. In more serious cases, this could lead to resource leakages. The more correct way to do this, would be to wrap each clean-up statement individually in its own try-catch block.

FileInputStream in = null;
OutputStream out = null;

try
{
    // Do Stuffs Here
}
finally
{
    if (in != null)
    {
        try
        {
            in.close();
        }
        catch (IOException ignore)
        {
            // Log Exception
        }
    }

    if (out != null)
    {
        try
        {
            out.close();
        }
        catch (IOException ignore)
        {
            // Log Exception
        }
    }
}

4. Throwing ExceptionS.

Throwing exceptions in general is not a big issue, it is the throwing of an instance of the Exception class (as opposed to a specific sub-class) that sometimes causes a loss of clarity in the code.

private String readFromFile(String filename) throws Exception
{
    FileInputStream in = null;

    try
    {
        in = new FileInputStream(in);

        // Do Stuffs
    }
    finally
    {
        if (in != null)
        {
            try
            {
                in.close();
            }
            catch (IOException ignore)
            {
                // Log Exception
            }
        }
    }
}

In the readFromFile method above, there are possibly FileNotFoundExceptionS and IOExceptionS that need to be handled. One way to handle them would be to catch them within the method itself (and appropriately handling them). Declaring throws FileNotFoundException, IOException in the method header is fine too, but declaring an all-purpose throws Exception is not a good practice, and there are a couple of reasons to this.

Firstly, the throws clause in a method header is considered to be part of the interface contract. Following the principles of abstraction and encapsulation, declaring specific exceptions (and providing the corresponding Javadoc comment) would allow a programmer to understand how this method could potentially fail without having to delve into the implementation details. Declaring a generic throws Exception simply tells the programmer that, well, this method could potentially fail.

Secondly, declaring specific thrown exceptions would allow the calling method to handle each exception separately in different catch blocks. In the example above, the calling method would have to handle the exception in a generic manner. Should it be more appropriate for the calling method to throw the exception instead of handling it, it would also have to declare a generic throws Exception.

5. Throwing the wrong exception.

private void setValue(String s) throws ClassNotFoundException
{
    if (s == null)
    {
        throw new ClassNotFoundException();
    }

    this.value = s;
}

This is a rather contrived example. The next example is a more common sighting, but it is not much more better off either.

private void setValue(String s) throws Exception
{
    if (s == null)
    {
        throw new Exception();
    }

    this.value = s;
}

This sort of follows the argument in the previous point about the importance of the declared exception. Always throw the appropriate exception type. If an appropriate exception is not available, by all means create a new sub-class of Exception. In the examples above, the most appropriate exception to throw would perhaps be the IllegalArgumentException.