Java: error handling with try-catch, empty-try-catch, dummy-return
The search engine uses a recursively defined function that throws exceptions easily. I tried 3 ways to handle exceptions:
- ignore with empty try-catch ()
- add-dummy-return stop err-propagation due to exeption
- throw a specific exception. (This part I don't really understand. If I leave, except, can I make it continue elsewhere without continuing the old excluded path?)
Some exceptions I don't really care, like the runtime -exception (NullPointer) of deleted files, but some I really like unknown things.
Possible exceptions:
// 1. if a temp-file or some other file removed during execution -> except.
// 2. if no permiss. -> except.
// 3. ? --> except.
The code is very import for the entire program. I used to add clittered-checks, try-catch, avoided-empty-try-catch, but it really blurred the logic. Some stoned result would make the code much easier to maintain. It was very frustrating to keep track of random exceptions from accidentally deleting a temporary file! How would you handle exceptions for the critical part?
the code
public class Find
{
private Stack<File> fs=new Stack<File>();
private Stack<File> ds=new Stack<File>();
public Stack<File> getD(){ return ds;}
public Stack<File> getF(){ return fs;}
public Find(String path)
{
// setting this type of special checks due to errs
// propagation makes the code clittered
if(path==null)
{
System.out.println("NULL in Find(path)");
System.exit(9);
}
this.walk(path);
}
private void walk( String path )
{
File root = new File( path );
File[] list = root.listFiles();
//TODO: dangerous with empty try-catch?!
try{
for ( File f : list ) {
if ( f.isDirectory() ) {
walk( f.getAbsolutePath() );
ds.push(f);
}
else {
fs.push(f);
}
}
}catch(Exception e){e.printStackTrace();}
}
}
The code is refactored from here.
a source to share
This is the most readable code I can do:
import java.util.*;
import java.io.*;
public class Find {
List<File> files = new ArrayList<File>();
List<File> dirs = new ArrayList<File>();
List<Exception> excs = new ArrayList<Exception>();
public Find(String path) {
walk(new File(path));
}
void walk(File root) {
for (File child : getChildren(root)) {
if (isDirectory(child)) {
dirs.add(child);
walk(child);
} else if (isFile(child)){
files.add(child);
}
}
}
(cont.)
boolean isDirectory(File f) {
try {
return f.isDirectory();
} catch (SecurityException e) {
excs.add(e);
return false;
}
}
boolean isFile(File f) {
try {
return f.isFile();
} catch (SecurityException e) {
excs.add(e);
return false;
}
}
List<File> getChildren(File root) {
File[] children;
try {
children = root.listFiles();
} catch (SecurityException e) {
excs.add(e);
return Collections.emptyList();
}
if (children == null) {
excs.add(new IOException("IOException|listFile|" + root));
return Collections.emptyList();
}
return Arrays.asList(children);
}
}
Here are some key observations:
- No need to check if there is
path
null
-
File(String pathname)
throwsNullPointerException
ifpathname == null
-
- No need to go from
String
toFile
toString
etc. like the source code.- Just work on
File
instead
- Just work on
- Effective Java 2nd Edition Item 25: Prefers Array Lists
- Potentially throwing methods are
File
encapsulated in non- throwing helper methods- The main logic of the recursive part is clean this way
-
File.listFiles()
,File.isFile()
andFile.isDirectory()
, eachthrows SecurityException
- It turns out that instead of throwing
IOException
, itlistFiles()
will returnnull
instead- This is manually converted to
IOException
- This is manually converted to
- It turns out that instead of throwing
- If an exception is thrown, just return something that won't get in the way
walk
- empty list from
getChildren()
-
false
fromisFile(File)
andisDirectory(File)
- empty list from
-
catch (Exception e)
bad at all, that's why we onlycatch (SecurityException e)
- Instead,
excs.add
you can use the registration framework to log the exception instead
a source to share
Ignoring an empty catch exception is usually dangerous. You must make sure the exception you catch is irrelevant to execution.
To clean up the method logic, you can retrieve the error handling code in another way. There you can put all the code you need to identify the source of the error and add it up if necessary.
catch(Exception e){
handleException();
}
private void handleException throws Exception() {...}
If you are interested in tracking exceptions in recursion, you can wrap the list in the parameters of your method to stack exceptions and handle them as soon as execution completes.
private void walk(String path, List<Exception> listExceptions) {...}
This way, you can ignore the error in the subpath by tracking it down and continue running through the rest of your tree.
a source to share
Depends on how you want to handle errors.
If you are currently browsing a directory of 100 files with your code and the second file throws an exception, what happens? Well, you will get a stacktrace in System.out, the walk method will complete and nothing else will happen. Find.getF () will only contain the first file, and the rest of your program will not know that something went wrong.
Thats probably not the way you want?
If you know you don't want some errors (like file not found) then put a try / catch block to do this inside your loop. In the catch block, you simply log what exactly went wrong with that particular file, and then you continue in your loop. Often you don't want to log full stacks, there is only one line here.
If you want to handle unexpected exceptions in some way, first determine how you want to handle them (just register and continue with the following file: send an email message? Show user dialog? End the program?) And then decide which class should are responsible for handling unexpected errors.
If you find that the caller of your class must handle unexpected errors, then catching and rethrowing your own Exception is a good way to tell the caller that something went wrong. If you decide that your Find class should handle unexpected errors, then put the handling in your catch block. If you want the loop to continue even after unexpected errors, remove your outer try / catch and make all the traps inside the loop.
a source to share
Comparison of Poly-SO and Apache Commons FileUtils connection code: iterateFiles and listFiles
Apache commons-io has similar iterateFiles and listFiles methods in FileUtils as suggested by Bozho . Parameter validation is done in various ways, but never "System.exit (9)"! They compare against null values, check for its existence using the File type (a method available to it). They use static, linkedList in the listFiles implementation - suggested in the poly book .
They reuse the field to match all dirs:
TrueFileFilter.INSTANCE True filter syntactic instance (from Apache API, singleton?)
The two methods are the only methods that take IOFileFilter as a parameter. I'm not sure about its implications. They can certainly reuse their code.
There are some very succinct - I think good - evaluation points, without blurring the dummy vars. Please take a look at the score (a? B: c), keep dumb thoughts and if-clauses.
return listFiles(directory, filter,
(recursive ? TrueFileFilter.INSTANCE : FalseFileFilter.INSTANCE));
The FileUtils class that contains these methods has only 4 field values - about 2.5 methods per field! Now I am ashamed of my class. The clear difference is in the use of Exceptions. They use them, but apparently because of the different purpose of the FileUtils class - they allow the user to process them, and there is no centralized collection in the list. No additional ads.
Summary
- Similarity: linkedList and list
- Differences: fewer inits, fewer decs, lower field density for methods - succincy
- Various targets: SO class end user case, FileUtils is more of a backend
- Difference (natural): Exception handling in SO but not in FileUtils (maybe that's why it's so clean).
I liked the comments and in particular the source - much better for educational purposes than I read trivial APIs. I hope you do too :)
Apache Commons: FileUtils.java, listFiles, iterateFiles - code snippets
/**
* Finds files within a given directory (and optionally its
* subdirectories). All files found are filtered by an IOFileFilter.
* <p>
* If your search should recurse into subdirectories you can pass in
* an IOFileFilter for directories. You don't need to bind a
* DirectoryFileFilter (via logical AND) to this filter. This method does
* that for you.
* <p>
* An example: If you want to search through all directories called
* "temp" you pass in <code>FileFilterUtils.NameFileFilter("temp")</code>
* <p>
* Another common usage of this method is find files in a directory
* tree but ignoring the directories generated CVS. You can simply pass
* in <code>FileFilterUtils.makeCVSAware(null)</code>.
*
* @param directory the directory to search in
* @param fileFilter filter to apply when finding files.
* @param dirFilter optional filter to apply when finding subdirectories.
* If this parameter is <code>null</code>, subdirectories will not be included in the
* search. Use TrueFileFilter.INSTANCE to match all directories.
* @return an collection of java.io.File with the matching files
* @see org.apache.commons.io.filefilter.FileFilterUtils
* @see org.apache.commons.io.filefilter.NameFileFilter
*/
public static Collection listFiles(
File directory, IOFileFilter fileFilter, IOFileFilter dirFilter) {
if (!directory.isDirectory()) {
throw new IllegalArgumentException(
"Parameter 'directory' is not a directory");
}
if (fileFilter == null) {
throw new NullPointerException("Parameter 'fileFilter' is null");
}
//Setup effective file filter
IOFileFilter effFileFilter = FileFilterUtils.andFileFilter(fileFilter,
FileFilterUtils.notFileFilter(DirectoryFileFilter.INSTANCE));
//Setup effective directory filter
IOFileFilter effDirFilter;
if (dirFilter == null) {
effDirFilter = FalseFileFilter.INSTANCE;
} else {
effDirFilter = FileFilterUtils.andFileFilter(dirFilter,
DirectoryFileFilter.INSTANCE);
}
//Find files
Collection files = new java.util.LinkedList();
innerListFiles(files, directory,
FileFilterUtils.orFileFilter(effFileFilter, effDirFilter));
return files;
}
/**
* Allows iteration over the files in given directory (and optionally
* its subdirectories).
* <p>
* All files found are filtered by an IOFileFilter. This method is
* based on {@link #listFiles(File, IOFileFilter, IOFileFilter)}.
*
* @param directory the directory to search in
* @param fileFilter filter to apply when finding files.
* @param dirFilter optional filter to apply when finding subdirectories.
* If this parameter is <code>null</code>, subdirectories will not be included in the
* search. Use TrueFileFilter.INSTANCE to match all directories.
* @return an iterator of java.io.File for the matching files
* @see org.apache.commons.io.filefilter.FileFilterUtils
* @see org.apache.commons.io.filefilter.NameFileFilter
* @since Commons IO 1.2
*/
public static Iterator iterateFiles(
File directory, IOFileFilter fileFilter, IOFileFilter dirFilter) {
return listFiles(directory, fileFilter, dirFilter).iterator();
}
// **** Cut out the ****** part //
/**
* Finds files within a given directory (and optionally its subdirectories)
* which match an array of extensions.
*
* @param directory the directory to search in
* @param extensions an array of extensions, ex. {"java","xml"}. If this
* parameter is <code>null</code>, all files are returned.
* @param recursive if true all subdirectories are searched as well
* @return an collection of java.io.File with the matching files
*/
public static Collection listFiles(
File directory, String[] extensions, boolean recursive) {
IOFileFilter filter;
if (extensions == null) {
filter = TrueFileFilter.INSTANCE;
} else {
String[] suffixes = toSuffixes(extensions);
filter = new SuffixFileFilter(suffixes);
}
return listFiles(directory, filter,
(recursive ? TrueFileFilter.INSTANCE : FalseFileFilter.INSTANCE));
}
/**
* Allows iteration over the files in a given directory (and optionally
* its subdirectories) which match an array of extensions. This method
* is based on {@link #listFiles(File, String[], boolean)}.
*
* @param directory the directory to search in
* @param extensions an array of extensions, ex. {"java","xml"}. If this
* parameter is <code>null</code>, all files are returned.
* @param recursive if true all subdirectories are searched as well
* @return an iterator of java.io.File with the matching files
* @since Commons IO 1.2
*/
public static Iterator iterateFiles(
File directory, String[] extensions, boolean recursive) {
return listFiles(directory, extensions, recursive).iterator();
}
a source to share