Java best practices 2 – Explicit cases
15 augustus 2007, door arjanThis is the second installment of my discussion about various bad practices in Java that I encountered during my work. As outlined in the first installment, this entry will be about “Not structuring different cases explicitly”.
After the first installment some readers wondered why the discussion is called “best practices”, while I actually talk about “bad practices”. The idea here is that recognizing these bad practices helps you in avoiding them and doing the opposite, which is a good practice 
Not structuring different cases explicitly
A particularly nasty bad practice is when programmers don’t structure different cases in their code simply as, well… different cases. Oftentimes this bad practice is introduced into a software system whenever an extension is made to existing code.
We all know the deal; we’ve created a nice and simple Servlet that only takes an ID of something and does some work with that in a clean and straightforward way. Inevitably however a boss or customer comes along, asking for an addition to be made. Now how do you handle this?
A beginning or perhaps less talented developer tends to just keep adding parameters to the URL calling the Servlet, sorting the now implicit cases out as the code progresses. At first this may seem reasonable, but it very soon becomes a total maintenance nightmare. Bug fixing becomes hard (which set of parameters belongs together?) and refactoring becomes near impossible if you need to unravel the tightly knitted fabric of 20 or more possible lines of execution, just to find out what cases the code actually handles. After some given threshold is reached even the original programmer is unable to make any changes at all to the code and development grinds to a halt.
If you ever come to work somewhere and a ‘senior’ developer tells you some piece of code can’t be touched since “it’s to dangerous to make changesâ€, it’s often because of exactly this bad practice.
To give you some idea of what this would look like in practice, take a look at the following code example. Let’s suppose a Servlet can be called using a URL with the following parameters:
“ownerID, customerID, productID, salesDescription, changeText and changeIDâ€
Now suppose the code handling these would look something like this:
processor.customerID = customerID;
if (ownerID != null) {
store.setOwnerID( ownerID );
// lots of other code ...
int foo = processor.getFoo(); // introduce an intermediate variable
// again lots of other code ...
if ( changeID != null && changeText == null && customerID > 1) {
store.setCustomerRegularID( customerID );
// lots of other code ...
if ( productID != null ) {
}
}
else if ( salesDescription != null && foo != changeID ) {
// again lots of code here
}
store.setFoo(foo);
// More and more code
}
// Lots of other code again ...
if ( productID != null && changeID != null ) {
// ...
}
// etc etc etc
This already looks pretty bad, but now suppose the “lots of other code†comment is actually replaced with lots of other code. It shouldn’t require too much imagination to understand that it becomes ‘rather difficult’ then to decipher what the code is doing.
The problem here is clear; every ‘command’ given to the code is implicitly expressed through a complex combination of overlapping parameters. Which combination of parameters relates to which command is extremely hard to grasp just by looking at the code. The above code may actually do relatively straightforward things such as “update product description†or “update customer description†but we just can’t see that when looking at the code.
The solution to this problem is equally straightforward; simply adopt the command pattern (described by the GOF in the most excellent book Design Patterns, Element of Reusable Object-Oriented Software). Using this pattern, the above code would look more like this:
switch (command.cmd) {
case updateProductDescription:
handleUpdateProductDescription(command.params);
break;
case updateCustomerDescription:
handleUpdateCustomerDescription(command.params);
break;
// other cases
}
A similar approach can be used at the URL level. Simply introduce one extra parameter called “cmd†and clearly document the meaning of the rest of the parameters depending on the value of the “cmd†parameter. E.g. compare:
http://example.com/foo?productID=4&description=some_description
with
http://example.com/foo?cmd=updateProductDescription&productID=4&description=some_description
This example may look trivial, but imagine 10 URLs with each a different combination of the parameters mentioned earlier. You’ll appreciate the cmd parameter pretty soon. Please note though that in object oriented frameworks like JSF we rarely need to construct URLs manually like this.
It may be hard to believe, but there’s actually an even more hideous form of the “Not structuring different cases explicitly†bad practice; Variable name re-using. This can actually be a bad practice by itself, but it most often shows up in combination with the former. Variable name re-using is often introduced into a software system when the number of parameters and conditionals in the code has already reached a certain threshold due to the usage of the implicit cases as described above. At this point the developer in question thinks he’s being clever and ‘abstracts’ a number of (partly) common cases by reusing existing variables to hold wildly different things. Of course, this only creates an even bigger mess.
E.g. imagine the first code fragment above starting with this:
if ( customerID != null ) {
ownerID = customerID;
}
if ( changeText != null ) {
ownerID = productID;
}
if ( salesDescription != null && changeText != null ) {
customerID = ownerID;
ownerID = salesDescription;
}
// Rest of the code as given in the first fragment here
Seems totally insane? The code fragment above is in fact a ‘simplified’ version of live code that I actually encountered during code auditing.
Well, that’s it for today. In the next installment we’ll be talking about “eating up exceptions” and “mixing JSTL and JSF for common cases”.
Arjan Tijms