Monday, February 10, 2014

Coding style

Style has an important role when it comes to writing clean, readable code. Conforming to conventions and best practices is a good starting point. In addition to that there is still plenty of room for choice. Those depend greatly on the level of the developers and personal preferences.

There is a very specific danger with conventions and best practices. Most of them are distilled experience of the industry and are there for good reasons. Some of them may be questionable but are rarely - if ever - questioned. They get incorporated into the coding style and without conscious effort to pinpoint them become invisible. I think all developers should investigate their code from time to time and inspect even the most trivial elements for such issues.

A couple of things that I think are worthy to look at:

Getter/setter methods
Since IDEs can generate them so easily, these are very often abused. Some developers have the habit of generating them right away, throwing away the most important element of object oriented programming - encapsulation - without further thought. Mutators are created on objects that could be immutable. The get...() prefix is often meaningless, and actually harms readability:
Constant naming conventions
The convention is to use uppercase names which can be quite unreadable - especially if there are many long-named constants in a class. Unless it's part of the api, the fact that a field is constant is an implementation detail.

Final keyword on local variables
Some static code analyis tools show warnings when local variables are not marked as final - if they've been assigned only once. While it adds value in some specific cases, if applied on all local variables without reason, the clutter it introduces can make the code harder to read. One can argue their rigorous use is good in long and complex methods, then the problem is the complex method in the first place.

Exception suffix
A really controversial one, but I think it's worth to at least toy with the idea. Almost all exceptions have the ...Exception suffix in their names. This is actually not really necessary and it does add some noise. Note that in whatever context we encounter an exception, the syntax makes it already obvious:
  • Definition of an exception - Something extends Exception
  • Throwing an exception - throw new Something
  • Declaration of an exception thrown - throws Something
  • Catching an exception - catch(Something)
Dropping the suffix can work on exceptions that are named as events, not just plain nouns - this is btw a useful practice on it's own right. Compare FileNotFound vs FileNotFoundException. Event-based names and lack of suffix applied throughout can have nice effects in a codebase. Some libraries do this, eg javassist, bouncycastle.

These are just some specific examples, the important part is to notice things we just do automatically and be concious about the reason why we're doing them, making sure they actually make sense.

1 comment:

  1. Hey I do agree with most of your thoughts except the Constant naming convention one, I think making constant be upper case really improve the code readability. If I were the maintainer, I am happy to see the code following the convention. The other thing is getter and setters, I don't like these java bean spec at all. However tools like Jackson/velocity etc relies on the getter and setter to manipulate on the java data objects. I'd say getter/setter are designed more for tools than for human beings.