Critique my exception handling strategy
exception, java
Solution
A few remarks:
When wrapping an exception e, you just use the e.getMessage(). Instead I suggest you use use new Exception(message, e). This will set the cause of the exception, so the entire stacktrace will also contain a stacktrace of the original exception. This can be very useful for debugging the cause of your exception.
Instead of throwing Exception, I suggest you throw more explicit exceptions, e.g. FileNotFoundException. Use the exceptions defined by the Java API when they makes sense.
Instead of catching Exception, I suggest you explicitly state which exceptions to catch.
(Optional replacement for 2) Some developers prefer to - instead of defining an exception class for each kind of exception - throw a general exception containing an error key indicating the type of the exception, e.g. new DetailedException(GeneralBusinessErrors.PARSING_ERROR). This makes it easier to assign language dependent resource files for your business errors.
It may be useful to add debug data to your exception. For example, if a file was not found, it could be information such as which file you tried to open. This is very useful in maintenance where you may be unable to reproduce the problem and/or debug using a debugger.
It may be useful to have your system logging any thrown unchecked exceptions that are not caught. Generally speaking, it may be useful to log any exception that is thrown (caught or not).
Problem
I've been doing object-oriented programming for most of the past 7 years, using Java on and off during that time. Some things I'm certain I have an excellent grasp of, such as the most useful design patterns. In fact, the following code allowed me to crank out a little system in under a day's time, that would handle the one particular instance we are ready to implement now, while being flexible enough to handle the future requirements I've been informed of: ``` public void importAndArchive(File detectedFile) throws FileNotFoundException, IOException { File workingCopy = returnWorkingCopy(detectedFile); List<String[]> csvData = csvData(workingCopy); setHeaderFields(csvData.get(0)); importData(csvData); //subclass will implement this abstract method archiveWorkingCopy(workingCopy); } ``` I don't show the above to boast about my grasp of the template method, but rather as a starting point to discuss a gap I have in my abilities that is glaring in light of my OO design abilities. And that gap is, a systematic approach to exception handling. Indeed you can see in that method signature I am for the time being re-throwing some exceptions, but actually I've gone on to eradicate the very same sort of thing in another corner of the application. Before I go much further though, since this is my first attempt to be especially systematic, I would like some validation of what I've done thus far. Our application loops over a bunch of files, "processes" them, and "archives" them. Pretty standard fare. One decision I've made in order to roll out a prototype as quickly as possible is as follows. The application gets initialized based on data in a (ResourceBundled) properties file. The various methods in the ResourceBundle API throw unchecked exceptions, but I am not really handling them whatsoever for the time being, on the grounds that they will prevent the app from starting, and the stacktrace can suffice for the time being. I have however chosen a library for CSV handling which throws checked exceptions. NetBeans then made it very easy to proliferate those exceptions to begin with ;) The following is a method that I've since reworked that actually handles those exceptions: ``` private String detectImportHandler(File detectedFile) throws Exception { String[] firstLine = null; try { /* * CSVReader throws checked exceptions */ CSVReader csvReader = new CSVReader(new FileReader(detectedFile)); firstLine = csvReader.readNext(); csvReader.close(); } catch(Exception x) { throw new Exception("CSVReader unable to process file: " + x.getMessage()); } try { return firstLine[1]; } catch(Exception x) { /* * since we're re-throwing CSVReader's checked exceptions it seems easiest * to also re-throw null-pointer and/or array-out-of-bounds errors */ throw new Exception( "First line null or did not have importHandlerType field: " + x.getMessage()); } } ``` The above method is being called thusly, inside a loop that processes the files: ``` try { importHandlerType = detectImportHandler(detectedFile); } catch(Exception x) { unusableFileErrors.put(detectedFile.getName(), x.getMessage()); continue; } ``` unusableFileErrors is a Map, and I figure when I am done iterating over the files, I can then use this map and the file-specific messages it contains to handle things at a higher level, such as logging, moving the files somewhere else on the system, etc. Anyway I've gone on long enough. I have ordered the book "Robust Java" and I'm hoping between it, and the SO community, I can improve this neglected facet of my abilities. I've seen other similar questions on SO but I figure that also requesting specific advice in the context of actual code might be of benefit.