Sometimes you just can’t find words to describe bad code, and if you are forced into maintenance it can be a mindfield. I’m presently supporting an existing deployed Web Based Java application, which I’ve had no involvement with previously, and for lack of any complements it’s absolutely terrible.
How this code was ever written, and of course never reviewed (that’s another talk on software development) one can only shutter. If it was an isolated case, then perhaps, but the code is bloated, and riddled with unnecessary complexity. Basic 101 practices such as Standard CSS, well formed html pages (ie, not with double <html> and <body> tags), and Javascript functions that are not duplicated up the warzoo are things you can discover spending 2 mins looking at just the first screen before you even look at the Java Code.
Here is an example.
tempKeyStr = tempKeyStr.substring(0,2).concat(String.valueOf(Integer.parseInt(tempKeyStr.substring(2).trim())));
Ok, I guess I should give you a hint of the type of values in the tempKeyStr prior to this wonderful transformation. This string contains a 2 alphanumerical character prefix, then optionally a space, then a numeric number (usually 2, 3 or 4 digits). An example may be ‘XX 999′ or ‘AA99′.
This will help you to identify what this lines now does?
Ok, well to save the suspense, the purpose of the above line of code is to trim and squeeze any whitespace from the string. So something like the following would surfice.
tempKeyStr = tempKeyStr.replaceAll(" ","");
Of course refactoring would totally eliminate this line, placing the replaceAll syntax in a previous declaration.
Those observant Java guru’s would also state, but what if this was written 5 years ago, replaceAll is only available since 1.4. Yes, correct, but Pattern has always existed, replaceAll is just a convience wrapper for this, and this type conversion from String to Integer to int to String is unnecessary complexity.
On that note of refactoring, I was sent the following link recently Refactoring to the Max. As the title suggests, sometimes people go to far.