So, by now everybody got it: Code is there to be read by people, to be analyzed by people and to be understood by people. The fact that you can put it through a compiler and run it is a nice sideffect, but nothing to focus on. Besides writing readable tests of course.
But when software is growing and many different hands touch the same spots, it somehow gets dirty. So even when you have usually quite high coding standards, it still can happen that I stumble upon something like this:
User user = userService.createNewUser( email, password,
false, true, null, 5 );
I like to be able to read a line of code like a line of text. Which means that I at least want to be able to get the “words” without the surrounding context.
So what would you be able to tell about the above snipplet? I would say: it apparently creates a new user, using some service and it requires an email and a password, which might be stored somewhere. And the point that makes me cry: apparently some magic flags, a nullable parameter and a magic number.
So lets see how we can clean this up with some new patterns. I usually make up my own names, if someone has a rather more common name for them feel free to comment, I will be happy to replace them.
Enum as Flag Pattern (aka do not use boolean literals)
enum FailIfExists { YES, NO };
enum NewPasswordChecks { YES, NO }
User user = userService.createNewUser( email, password,
FailIfExists.NO,
NewPasswordChecks.YES,
null, 5 );
So, what can you tell about the method call now. The problem of methods that react to flags aside, the readability is better. You at least can deduct now that there might be no error if the user already exists and that it uses the “new” password checks. It is even easier to refactor to use the third password validation algorithm should it ever be changed again.
When setting up a new project you might try to disallow all boolean literals in certain high level classes, I don’t know if it works that well with third party libraries. This might be a cool Checkstyle rule to try.
No nullable Parameters Pattern (aka keep your implementation details internal)
So what is my next problem? Well, the given null parameter value. I believe that the ability to cope with null values is an implementation detail that belongs in the class that defines a method. So the UserService interface should provide us with an overridden version createNewUser that does not have the fifth parameter. Then the implementation could hide the fact that this parameter is really nullable. And it avoids the clutter of methods that have n optional object parameters.
If you use Findbugs in combination with the great JSR 305 annotations, which by the way can be enforced using a Checkstyle plugin, you might try to disallow using the Nullable Annotation in public methods. Maybe even for protected methods. In any case, you should never have to use a null parameter while calling a visible method of another class.
No literals outside constant definitions (aka give names to your magic numbers)
The last thing is a classic, but there are still a couple of people who do not use constants instead of literals. I think the general rule is that you should never use a numeric literal inside a method, but always a class declared constant. Furthermore this might even be extended to be valid for String literals and as told in the first point to boolean literals.
So let’s revisit my short (and bad) example taking the above mentioned points into consideration:
enum FailIfExists { YES, NO };
enum NewPasswordChecks { YES, NO }
private static final int INITIAL_NUMBER_OF_INVITES = 5;
User user = userService.createNewUser( email, password,
FailIfExists.NO,
NewPasswordChecks.YES,
INITIAL_NUMBER_OF_INVITES );
Given that you might see this lines of code during a review, where you can’t browse the complete source of your project, one might now be able to better understand what this method does: it creates a new user with the given email and passwords, succeeds even when the user already exists, uses some mythical new password check and gives him an initial number invites of five. This can be guessed by just reading code, without a single line of JavaDoc and even without once visiting the declaring class or interface.
So in future, when writing code, try to consider if you would understand your method calls without ever having seen the implementation or even the declaration of the called method.
blog.thiesen.org 