tags:

views:

157

answers:

7

I am new to exceptions, haven't covered them in college yet so still learning about them. I tried this and it seems to work, but doesn't seem "right". What's the correct way to try a method again after a exception has been handled?

public static void openCSV(String file) {
    FileInputStream fis;

    try {
        fis = new FileInputStream(file);
    } catch (FileNotFoundException e) { //fnf, probably not downloaded yet.

        downloadCSV(file); //Download it and try again.

        try {
            fis = new FileInputStream(file);
        } catch (FileNotFoundException e) {
            // OK, something else is the problem.
        }
    }
}
A: 

In that scenario, you're using exception handling incorrectly for the first block.

You can simply check whether the file exists first time around, then attempt to download it.

Wim Hollebrandse
+4  A: 

Your question is not per se about "exceptions", but is more about design in general. You'll get many, many opinions about the right way to handle this.

The most obvious fix is

if (!new File(file).exists()) {
    downloadCSV(file);
}
try {
    fis = new FileInputStream(file);
} catch (IOException e) {
    // scream
}
Jonathan Feinberg
Note that `file` is `String` his case. He'll need to construct `File` first.
BalusC
Many thanks, BalusC. Edited.
Jonathan Feinberg
Don't forget to warn about potential troubles with relative paths as well ;) +1 nevertheless.
BalusC
+2  A: 

I like to try to avoid using exceptions like that where I can help it. This is one such case:

public static void openCSV(String file) {
    FileInputStream fis;

   if (!(new File(file).exists())) {
        downloadCSV(file); //download it and try again
    }

    try {
        fis = new FileInputStream(file);
    } catch (FileNotFoundException e) {
        // ok something else is the problem;
    }
}
Dan
A: 

While your particular example is probably not what you would normally do for checking if a file exists, your methodology is correct. In general, you want to catch the exception, probably try again, if the same error occurs, tell the user it failed and that they should try something else or contact support.

Topher Fangio
+1  A: 

You should probably call downloadCSV(file); outside openCSV(file). If FileNotFoundException exception is caught you should re throw to the caller. You should also use a finally block to close stream.

fastcodejava
+2  A: 

This is a form of exception misuse. If sometimes the file must be downloaded, you shouldn't rely on an exception to tell you so.

Try something like this:

public static void openCSV(String file) {
    FileInputStream fis;

    try {
        if (!(new File(file).exists())) {
            downloadCSV(file); //download it
        }
        fis = new FileInputStream(file);
        // should probably use the stream here, so you can close it in a finally clause linked to this try clause...
    } catch (FileNotFoundException e) { //file doesnt exist
        // Re-throw as a declared exception, a RuntimeException, and/or log it...
    } finally {
        if (fis != null) {
            try {
                fis.close();
            } catch (IOException ioe) {
                // write exception to logs or take other appropriate action...
            }
        }
    }
}
Drew Wills
`fis.close()` should be in another `try/catch` block as well.
BalusC
BalusC is correct. edited.
Drew Wills
A: 

I don't see a problem with the code. (Other than I would put it in its own method; don't use made up acronyms like fis; openCSV should probably throw an exception and probably shouldn't be static.)

The obvious reason why you don't want to use File.exists is that two operations on the file path are not going to be atomic. That's a common problem with file operations. Keep things atomic as much as possible (note, this works against "do only one thing"). Two operations may well also be slower than one.

Also, File.exists might not be able to determine whether the file exists or not. You might not have permissions to read the directory, for instance (minimal permissions are a good thing).

Tom Hawtin - tackline