CODEWITHSUNDEEP
javaexceptionrobustness
chustar
chustar

Reputation: 12465

How do I open, write and save a file while avoiding errors?

I need to erase a file in my program. My solution was to have an erase() method that would do that like so:

public static void erase(String string) {
    FileWriter fw = null;
    try {
        fw  = new FileWriter(string);
        fw.write(new String());
    } catch (IOException ie) {
        e.printStackTrace();
    } finally {
        fw.flush();
        fw.close(); 
    }
}

Several problems here:

What other problems am I overlooking and how can I harden this method?

Upvotes: 0

Views: 267

Answers (3)

L. Cornelius Dol
L. Cornelius Dol

Reputation: 64026

Include the flush() in your main block and have only the close() in the catch. Then check for null before closing:

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

With the flush in the main block, you can also try/catch the close and log or ignore any error:

finally {
    if(fw!=null) { 
        try { fw.close(); } catch(Throwable thr) { log.printError("Close failed: "+thr); thr.printStackTrace(); } 
        }
    }

or (not generally recommended):

finally {
    try { fw.close(); } catch(Throwable thr) {;}
    }

EDIT

The best general Java idiom for handling I/O, IMO, is:

FileWriter fw=null;
try {
    fw=new FileWriter(string);
    fw.write(new String());
    fw.close();
    fw=null;
    }
catch(IOException ie) {
    // do something real here to handle the exception, or don't catch it at all.
    } 
finally {
    if(fw!=null) { 
        try { fw.close(); } catch(Throwable thr) { thr.printStackTrace(); } // now we're really out of options
        }
    }

This has the important effect of allowing the catch to catch and handle an exception thrown by close() itself. (The catch clause should only be present if you can handle the exception in some way; do NOT catch and ignore, and generally you should not simply catch and trace.)

Upvotes: 1

Nayuki
Nayuki

Reputation: 18533

To the best of my knowledge, this is the correct idiomatic way to write your code:

FileWriter fw = new FileWriter(string);
try {
    fw.write(new String());
    fw.flush();
} catch (IOException ie) {
    ie.printStackTrace();
} finally {
    fw.close(); 
}

Explanation:

  • If new FileWriter() throws an exception, then we don't need to clean up anything. The method exits without executing the finally.
  • We should put fw.flush() in the try, not in the finally. There are two reasons: If the write failed, then we shouldn't bother flushing. Also, if you put the flush() in finally and it throws an exception, then the close() will be skipped.

Upvotes: 1

dhg
dhg

Reputation: 52681

You can just wrap the finally functionality in an if-statement:

if(fw != null){
    fw.close(); 
}

This will ensure that if the file was ever opened, then it will be closed. If it wasn't opened in the first place, then it won't do anything, which is what you want.

Also, I'm not sure if it's just like that for posting, but it's generally not advisable to just print a stacktrace in a catch block and continue ("swallowing" the exception). You really should let the exception get thrown because this could hide bugs and make tracking them down very hard.

EDIT: See comment below.

Upvotes: 3

Related Questions