tags:

views:

268

answers:

7

Usually, when dealing with Java IO code, here is what I wrote

    FileOutputStream out = null;
    try
    {
        out = new FileOutputStream("myfile.txt");
        // More and more code goes here...
    }
    catch (Exception e)
    {
    }
    finally 
    {
        // I put the close code in finally block, to enture the opened
        // file stream is always closed even there is exception happened.
        if (out != null) {
            // Another try catch block, troublesome.
            try {
                out.close();
            } catch (IOException ex) {
            }
        }
    }

As you can see, while I try to close the file stream, I need to deal with another try...catch block.

Look troublesome :(

Is there any way I can avoid? I don't feel comfortable in putting the close code in non-finally block, as exception caused by other codes will make no chance for "close" being called.

+4  A: 

Discussion at

http://stackoverflow.com/questions/1335812/try-catch-finally-and-then-again-a-try-catch

and

http://stackoverflow.com/questions/183499/is-there-a-preference-for-nested-try-catch-blocks

basically, the question is whether a close() exception is worth catching.

Sanjay Manohar
+1  A: 

I tend to use utility functions for this:

public static void safeClose(OutputStream out) {
  try {
    out.close();
  } catch (Exception e) {
    // do nothing
  }
}

which changes the code to the slightly more palatable:

FileOutputStream out = null;
try {
  out = new FileOutputStream("myfile.txt");
  // do stuff
} catch (Exception e) {
  // do something
} finally {
  safeClose(out);
}

You can't really do much better in Java at least until Java 7 when (hopefully) ARM ("Automatic Resource Management") blocks will help somewhat.

cletus
ARM block? May I know what is that?
Yan Cheng CHEOK
Some point to the safeClose. You should check for null pointer as well, right?
Yan Cheng CHEOK
@Yah added a link about ARM blocks to the post. As for checking for `null`, you can but it's largely superfluous. If you throw a `NullPointerException` it'll be caught in the `catch` clause anyway. But of course you can add that check if you wish.
cletus
"catch (Exception e)" is bad because it will mask runtime exceptions that should cause the process to die or be explicitly handled (and are very unlikely). Check for null, catch the checked exceptions is a better approach.
janm
+5  A: 

It is very important that you close streams in a finally. You can simplify this process with a utility method such as:

public static void closeStream(Closeable closeable) {
    if(null != closeable) {
      try {
        closeable.close();
      } catch(IOException ex) {
        LOG.warning("Failed to properly close closeable.", ex);
      }
    }
  }

I make it a point of at least logging a stream close failure. The usage then becomes:

FileOutputStream out = null;
try
{
    out = new FileOutputStream("myfile.txt");
    // More and more code goes here...
}
catch (Exception e)
{
}
finally 
{
    closeStream(out);
}

In Java 7 I believe that streams will be closed automatically and the need for such blocks should be mostly redundant.

S73417H
From what I hear on SO, Java 7 will also sleep in the wet patch.
kibibu
Did you mean "catch (IOException e)" instead of "catch (Exception e)"?
janm
It's a cut n' paste of the original example code from the user asking the question. But yes it should be IOException.
S73417H
+1 The only comment I would make is that I like to have LOG passed as an parameter to closeStream() as the log error should probably be associated in the logs with the caller. Not hugely important, especially @warn, but I like to keep the logs tidy.
Recurse
+3  A: 

Automatic Resource Management is coming in Java 7 which will automatically provide handling of this. Until then, objects such as OutputStream, InputStream and others implement the Closeable interface since Java 5. I suggest you provide a utility method to safe close these. These methods generally eat exceptions so make sure that you only use them when you want to ignore exceptions (e.g. in finally method). For example:

public class IOUtils {
    public static void safeClose(Closeable c) {
        try {
            if (c != null)
                c.close();
        } catch (IOException e) {
        }
    }
}

Note that the close() method can be called multiple times, if it is already closed subsequent calls will have no effect, so also provide a call to close during the normal operation of the try block where an exception will not be ignored. From the Closeable.close documentation:

If the stream is already closed then invoking this method has no effect

So close the output stream in the regular flow of the code and the safeClose method will only perform close if something failed in the try block:

FileOutputStream out = null;
try {
    out = new FileOutputStream("myfile.txt");
    //... 
    out.close();
    out = null;
} finally {
    IOUtils.safeClose(out);
}
krock
+1  A: 

Write a method that looks something like below; call from your finally block...

static void wrappedClose(OutputStream os) {
  if (os != null) {
    try {
      os.close();
    }
    catch (IOException ex) {
       // perhaps log something here?
    }
  }
seand
+1  A: 

Project Lombok provides a @Cleanup annotation that removes the need for try catch blocks all together. Here's an example.

Brian Harris
A: 

Separate your try/catch and try/finally blocks.

try
{
    FileOutputStream out = new FileOutputStream("myfile.txt");
    try
    {
        // More and more code goes here...
    }
    finally 
    {
        out.close();
    }
}
catch (Exception e)
{
    //handle all exceptions
}

The outer catch will also catch anything thrown by the close.

ILMTitan
The more code you put into a try block, the harder it is to handle exceptions in the correct way.
Darron
The only code added to the try block is the out.close(). I would argue that an error thrown by that probably needs to be handled the same way as any error caused by creating or using out.
ILMTitan