Thursday, September 27, 2012

Exception Handling

One of the surest signs of generally bad programming style, in my opinion, is improper exception throwing and handling.

Those of you who grew up with Java don't know how lucky you are to have a well-thought-out scheme of exceptions to deal with.   I wouldn't say that Java has perfect exception capabilities but they're reasonably close.

So, what is it that I think is so bad?

Here is an example of something that is just awful (but fortunately rare) -- I think everyone would agree:
public class BadExceptionHandling {

 public static void main(String[] args) {
  double total = 0;
  for (int i = 0; i < args.length; i++)
   try {
    total += Double.parseDouble(args[i]);
   } catch (NumberFormatException e) {
   }
  System.out.println("Total: " + total);
 }
}

I invoked this application with arguments 1, 2.5 and d4.  The d was a typo of course.  But instead of warning me that all is not well, the application gaily prints out:
Total: 3.5

The problem of course is in the "eating" of the NumberFormatException.  Why bother to catch it (it's actually a RuntimeException so you don't need to) if you're simply going to eat it?

Here's an improvement in the main method:


 public static void main(String[] args) {
  double total = 0;
  for (int i = 0; i < args.length; i++)
   try {
    total += Double.parseDouble(args[i]);
   } catch (NumberFormatException e) {
    e.printStackTrace();
   }
  System.out.println("Total: " + total);
 }

 The result is now:
java.lang.NumberFormatException: For input string: "d4"
    at sun.misc.FloatingDecimal.readJavaFormatString(FloatingDecimal.java:1241)
    at java.lang.Double.parseDouble(Double.java:540)
    at robin.BadExceptionHandling.main(BadExceptionHandling.java:22)
Total: 3.5

This is better -- just.  It shows that an exception was thrown.  But it still prints a total without a care in the world!  That "total" is simply invalid and should not be presented to the user at all.

However, at least in this case we were in control of the user interface (such as it is) because we were programming in a main method.  But, realistically, how many of you ever program main methods other than perhaps for testing purposes?  Basically never.  Everything that the average Joe Programmer does is meant to be invoked within a much larger system, a web application typically, but it could be a database, some sort of server or whatever.

The point is that when you are writing a method which is not a main method, you do not have the appropriate privilege to print stack traces or, worse, eat exceptions.  You have a responsibility to do something with the exception.  Most of the time, that something is simply passing it back up the stack frame.  But it could be wrapping it in another exception in order to provide more context regarding the circumstances of the error.  It should never be to print a stack trace, which will likely end up in some obscure log file, and carry on as if nothing had happened.

Let's see what I mean in the following where I have refactored by creating a method:
public class BadExceptionHandling {

 public static void main(String[] args) {
  try {
   System.out.println("Total: " + addArguments(args));
  } catch (Exception e) {
   e.printStackTrace();
  }
 }

 /**
  * @param args
  * @return
  * @throws Exception
  */
 private static double addArguments(String[] args) throws Exception {
  double total = 0;
  for (int i = 0; i < args.length; i++)
   try {
    total += Double.parseDouble(args[i]);
   } catch (NumberFormatException e) {
    throw new Exception("Problem adding argument " + args[i] + " to current total " + total);
   }
  return total;
 }
}

And the result is as follows:
java.lang.Exception: Problem adding argument d4 to current total 3.5
 at robin.BadExceptionHandling.addArguments(BadExceptionHandling.java:37)
 at robin.BadExceptionHandling.main(BadExceptionHandling.java:20)
I wouldn't normally throw an instance of Exception (I would typically sub-class Exception and throw a new instance of that) but that isn't important here.  What is important is that our main program was able to behave properly (simply show the stack trace without attempting a total).  And our method appropriately handles the NumberFormatException, and declares that the (checked) exception can be thrown.

Is that so very hard?

OK, back to work!

No comments:

Post a Comment