What to do with the Exception when you fail to close() a resource?

Question

I am happy to receive all recommendations for improvement. But below I am mostly interested in a review of my handling of the exception thrown by the close() method of RandomAccessFile.

The FileNotFoundException thrown by the opening of a file (in the constructor of RandomAccessFile) is declared as thrown by my method, because the caller can act on it.

But the exception thrown by the close() method on that file cannot be handled the caller, so I repackage it in a RuntimeException.

private boolean includeArticle(File articleFile, Map<String, String> conditionMap)
  throws FileNotFoundException 
{
  if (doesFileQualifyAsArticle(articleFile)) {
    /* This can throw a FileNotFoundException, which the caller should know about. */
    RandomAccessFile raFile = new RandomAccessFile(articleFile, "r");
    if (this.isToInclude(raFile, conditionMap)) {
      return true;
    }
    try {
      raFile.close();
    } catch (IOException e) {
      throw new RuntimeException(e);
    }
  }
  return false;
}

Answer 1

At least in code that I've seen, it is very common to ignore exceptions when closing a resource. In fact Apache Commons IO has a number of closeQuietly functions just for this purpose.

Also, the work being done with raFile should be wrapped in a try, with the close/closeQuietly being called in the finally. This ensures that close/closeQuietly is called even if an exception occurs while working with the file.

RandomAccessFile raFile = new RandomAccessFile(articleFile, "r");
try {
  if (this.isToInclude(raFile, conditionMap)) {
    return true;
  }
}
finally {
  IOUtils.closeQuietly(raFile);
}

Also, if you are using Java 7, you might want to check out the new try-with-resources statement.

Answer 2

From your description, I assume you only do reading. At least, you only mention FileNotFoundException, no write exceptions. If you do only reading, you can most likely do it in this way, with a small improvement:

....
finally {
    try {
        if (raFile != null)
            raFile.close();
    } catch (IOException e) {
        log.warn(e); // not interesting, we are in read only mode
    }
}

Otherwise, you could throw an additional NullPointerException.

If you need it several times, you could use a static utility method:

public static void closeQuietly(Closeable closeable) {
    try {
        if (closeable != null)
            closeable.close();
    } catch (IOException e) {
        log.warn(e); // we do not care
    }
}

(which is the same implementation as for apache commonsio)

If you do writing, you should throw the exception. Because the final close can lead to the same exceptions as every write (remember, we do probably a last flush before the close and write unwritten data). I would suggest to do only:

finally {
    if (closeable != null)
        closeable.close();
}

or rethrow the exception. This is exactly the same what happens with try-with, see for example here: http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html

And do not use abbreviations.