Java Language Common Java Pitfalls Pitfall: testing a file before attempting to open it.
Example
Some people recommend that you should apply various tests to a file before attempting to open it either to provide better diagnostics or avoid dealing with exceptions. For example, this method attempts to check if path corresponds to a readable file:
public static File getValidatedFile(String path) throws IOException {
File f = new File(path);
if (!f.exists()) throw new IOException("Error: not found: " + path);
if (!f.isFile()) throw new IOException("Error: Is a directory: " + path);
if (!f.canRead()) throw new IOException("Error: cannot read file: " + path);
return f;
}
You might use the above method like this:
File f = null;
try {
f = getValidatedFile("somefile");
} catch (IOException ex) {
System.err.println(ex.getMessage());
return;
}
try (InputStream is = new FileInputStream(file)) {
// Read data etc.
}
The first problem is in the signature for FileInputStream(File) because the compiler will still insist we catch IOException here, or further up the stack.
The second problem is that checks performed by getValidatedFile do not guarantee that the FileInputStream will succeed.
-
Race conditions: another thread or a separate process could rename the file, delete the file, or remove read access after the
getValidatedFilereturns. That would lead to a "plain"IOExceptionwithout the custom message. -
There are edge cases not covered by those tests. For example, on a system with SELinux in "enforcing" mode, an attempt to read a file can fail despite
canRead()returningtrue.
The third problem is that the tests are inefficient. For example, the exists, isFile and canRead calls will each make a syscall to perform the required check. Another syscall is then made to open the file, which repeats the same checks behind the scenes.
In short, methods like getValidatedFile are misguided. It is better to simply attempt to open the file and handle the exception:
try (InputStream is = new FileInputStream("somefile")) {
// Read data etc.
} catch (IOException ex) {
System.err.println("IO Error processing 'somefile': " + ex.getMessage());
return;
}
If you wanted to distinguish IO errors thrown while opening and reading, you could use a nested try / catch. If you wanted to produce better diagnostics for open failures, you could perform the exists, isFile and canRead checks in the handler.
