How to surround code with try/catch most elegantly

Question

I often have some problems when using try-and-catch:

1) Some variables need to be declared inside the try brackets otherwise they will not be in scope
2) Ultimately even my return statement ends up having to be in the try bracket but then the method doesn't return anything.

What is the proper way to get around this sort of problem.

An example of a method that is causing this issue is the one below. It neesd a FileNotFoundException to be handled and a IOException to be handled. How do I do this most elegantly?

public static String getContents (File file) {
   BufferedReader reader = new BufferedReader(new FileReader(file));
   String contents = new String();
   while (reader.ready())
    contents += reader.readLine();
   return contents;
 }

Answer 1

There is one more option if the exception handling doesn't need to happen in the getContents method -- add a throws clause to the method to make the method throw an exception:

public static String getContents (File file)
    throws IOException, FileNotFoundException {

This way, the code calling the method will handle the Exceptions rather than the method itself. There will be no need for try/catch blocks in that method, if the Exceptions are thrown up to the methods that called it.

This may or may not be the desired way to handle the situation, depending on how the method is expected to behave.

Edit

Upon second thought, making the method throw an exception may be a good idea. I think D.Shawley's comment I think sums it up well -- "exception handling should mean only handling the exception where it makes sense."

In this case, the getContents method appears to get the contents of a specified File, and returns a String to the caller.

If the exception handling were to be performed in the getConents method, the only way to communicate that an error has occurred is by returning some kind of pre-determined value, such as null to the caller to inform that an error occurred.

However, by having the method itself throw an exception back to the caller, the caller can choose to react accordingly:

try {
    String contents = getContents(new File("input.file"));
} catch (IOException ioe) {
    // Perform exception handling for IOException.
} catch (FileNotFoundException fnfe) {
    // Inform user that file was not found.
    // Perhaps prompt the user for an alternate file name and try again?
}

Rather than having the setContents method come up with its own protocol for notifying that an error occurred, it would probably be better to throw the IOException and FileNotFoundException back to the method caller, so the exception handling can be performed at a place where the appropriate alternate actions can take place.

Exception handling should only be performed if some meaningful handling can take place.

Answer 2

You can try to move your return statement into a finally block.

Answer 3

Regarding the variable scope, I not sure if there is a more elegant way to do it. Normally I try to think of what my return value is in case of an error, and then assign the variable to that value.

In regards to the return statement, if you use my above suggestion then, you can just return after the try / catch block.

So if I used a null return value to indicate an error I would do

public static String getContents (File file) {
    String contents = null;
    try {        
        BufferedReader reader = new BufferedReader(new FileReader(file));
        contents = new String();
        while (reader.ready())
            contents += reader.readLine();
    } catch (Exception e) {
        // Error Handling
    }
    return contents;
}

Answer 4

You can handle it as follows:

StringBuilder contents = new StringBuilder();
BufferedReader reader;

try {
   reader = new BufferedReader(new FileReader(file));

   while (reader.ready()) {
      contents.append(reader.readLine());
   }

} catch (FileNotFoundException fne) {
   log.warn("File Not Found", fne);
} catch (IOException ioe) {
   log.warn("IOException", ioe);
} 

return contents.toString();

You should probably use a StringBuilder in the case above instead of a String though, much better in terms of performance.

Answer 5

public static String getContents (File file) {
    String contents = new String();
    BufferedReader reader = null;
    try {
        reader = new BufferedReader(new FileReader(file));
        while (reader.ready())
            contents += reader.readLine();
    }
    catch (FileNotFoundException ex) {
        // handle FileNotFoundException
    }
    catch (IOException ex) {
        // handle IOException
    }
    finally {
        if (reader != null) {
            try {
                reader.close();
            }
            catch (IOException ex) {
                // handle IOException
            }
        }
    }
    return contents;
}

I added a finally block to close your BufferedReader, although you didn't do it in your code. I'd also suggest you use StringBuilder instead of String concatenation, but someone has already pointed it out. The declaration and inicialization of reader is outside the try block only because of the finally block I've added; otherwise, the reference reader could be declared inside the try block.

and I didn't handle the exceptions, I didn't think that was related to your question.

Answer 6

IMHO you have two ways to deal properly with the exceptions (here, IOException and FileNotFoundException) :

• you simply throw it, so the caller must deal with it, but in counterpart have the detailed reason of the failure and so can choose the most appropriate behavior
• you embed both possible exceptions in only one of yours, that express "something went wrong" so the caller have only one exception to deal with (but the end user message will probably be less acute)

To have good advice about exceptions, see also : http://stackoverflow.com/questions/921471/why-java-people-frequently-consume-exception-silently

Answer 7

What you're missing here is a simple utility which simplifies cleanup considerably:

public static void closeAndLog(Closable c) {
    if ( c == null )
        return;

    try { 
        c.close() 
    } catch ( IOException e) {
        LOGGER.warn("Failed closing " + c +, e);
    }
}

This way your code can become:

public static String getContents (File file) throws IOException {

    BufferedReader r = null;

    try { 
        r = new BufferedReader(...);
        // do stuff
    } finally {
        closeAndLog(r);
    }
}