Just because they do that, doesn’t mean you should
I spend my day job telling developers (and senior developers and “architects”(1)) to spend more time thinking about the design of their code. And when I say design I’m talking about using inheritence rather than copy/paste, or not throwing (or catching) java.lang.Exception all over the place. So it’s really basic stuff, it’s not rocket science.
That’s pretty annoying that I’m constantly correcting those kinds of mistake, but the worst part is when someone replies; but “Sun/Hibernate/Spring/fashionable-whizzy-framework does it like that.” That gets my goat for two reasons;
- It shows that people copy things without thinking about them
- Sometimes, the experts do daft things
Please understand what you’re copying
One of my most frequent comments when doing code reviews is;
Do not throw java.lang.Exception.
The response I get most often is
But org…..servlet.mvc.AbstractFormController does.
What does this tell me? This tells me that when someone extends the SpringMVC class, they just copy the method headers, fine. But it is not right to throw the base exception in implementation code. Java allows you to throw a narrower exception when you over-ride a method. So that’s what you should be doing, not just copying what the framework does.
(Framework is emphasised to reinforce the fact that it must do things slightly differently to allow it to be used and extended. Concessions are made for it. No concessions can be made for production implementation code.)
We can work out why the framework class does this, specifically so that implementing classes can throw narrower exceptions as makes sense to their design. But it seems like a lot of the developers just copy it out and carry on. Which makes me think; Do they do this because they don’t understand exceptions in relation to method over-riding? Do they not get why throwing Exception is bad? Are they lazy? Are they stupid? I’m not passing judgement on any of the developers I work with, I just wish that they’d think a little more about what they’re doing.
Sometimes, the experts do daft things
I apologise for the following, because it’s 100% rant. Let me give you an example of some really bad code that I’m not to scared to stand up and moan about. I hate java.lang.Math. It is not a well designed class, that probably comes from Java’s bizarre inclusion of primitives, but what is the point of a class that contains only static methods? It’s an awful throwback to procedural or library based code, and that’s not what Java is supposed to be.
A much (much much) better approach would to use, I dunno, say OO principles to supply this functionality; where everything is an object and the methods dictate their behaviours. For example, this means that this method;
public static double abs(double a);
Should instead belong to the Double class (as well as Integer, Long and Float) in the guise;
public void abs();
or possibly;
public Number abs();
depending on what kind of contract Sun decided was needed. Obviously there’s going to be some kind of overlap (maybe we can call it inheritence?) in the functionality provided across different types of Number, which begs the question… Why isn’t the abstract java.lang.Number class used more appropriately? It should be used to share out these kinds of methods and make sure that subclasses implement them appropriately. It should include a
/**
* Returns a random number that exists between the value of the
* current Number object and the input parameter inclusive.
*
* (That's quality, I even JavaDoc my imaginary methods!)
*
* @param boundary
* The limit of the random number (this can be less than the
* value of the object it is called on).
* @return
* A random Number between of the same type of the object
* it is called on between the value of the object and the
* input parameter inclusive
*
*/
public Number random(Number boundary) {
// implementation
}
method, which should be implemented for each kind of Number. That doesn’t seem like much but it sticks much more closely to OO principles and it negates the need for the developer to do the
int ranIndex = (int)(Math.random()*max);
just to get a random number.(2)
And while we’re at it why not have some useful Number based methods in the Number class. “Like what?” I hear you cry. Well ‘add’ would be a good one. Maybe ’subtract’ too. Hey, let’s be exotic and go for ‘divide’ and ‘multiply’ to. I mean, good grief, what’s the point of having these Number classes when they don’t actually have any useful number-based methods in them?!
Now Java has just been open sourced, so I can try submitting a patch but I can’t remember what license they put it out under and I don’t know what the process is for getting stuff included – or even if you can! But it’s something I can do that will make a difference and make something better. Maybe I’ll do that once I’ve finished these code reviews…
1 The inverted commas should imply what I think of some of the Architects I work with; but that’s another post
2 I know that the code is trivial, but using OO principles it can be made even more simple
