Why is it important to have a source format style?

Source formatting style seems like such a minor thing. I like one format for code, my colleague likes another style. Live and let live, right?

Unfortunately, it's not quite so simple as that, at least not when you might both be making changes in the same source code.

One of the trickier aspects of working as a team is that there will occasionally be conflicts in source code. This is especially true in an agile environment. Of course, you do your best to ensure that classes are as large as they have to be and no larger. That can help avoid unnecessary conflicts.

But there are likely to be some classes that are being frequently updated and if you and your colleague are both making changes, it's really important to agree on a common code format style. That's because differences even in white space can really complicate the set of conflicts and therefore make a merge much harder. And anything which is harder is more prone to errors.

Otherwise, let's say you make a change and check it in with your favorite formatting (X) applied. Meanwhile, your colleague is making changes using his favorite formatting (Y). When he comes to commit his changes, he's going to find that the file changed on him. He has to merge. He's going to hate it when there are lots of insignificant white space changes.

He's going to hate it even more if, for example, you've put in curly braces everywhere that they're legal and, under Y, all unnecessary curly braces are removed.  There are a million ways to adjust the formatting of source code.

So, pick one style, and set up Eclipse or your favorite IDE to apply the formatting changes whenever you save.  Then, before you commit your code, actually go in and do cleanup (you can run cleanup on your entire source tree at once).  If you always follow that plan, it will ensure that the changes (and conflicts) that show up are exactly what the difference should be -- neither more nor less.

OK, back to work!

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!