Monday, August 04, 2008

Four harmful Java idioms, and how NOT to fix them

In his article "Four harmful Java idioms, and how to fix them", John O'Hanley writes about how to make Java more maintainable. He picks four common patterns and gives tips how to fix them ... or not. Follow me.

Names

The first idea is to prefix names with a letter giving a hint what they mean: Is "start" a method? A field? A parameter? The goal is to make the code more readable to humans.

Unfortunately, this doesn't work. The human brain doesn't read letters, it reads words. So "fStart" (meaning a field with the name "start") is rejected by the brain because it's not a word. This triggers the conscious analysis which John tries to avoid! Which is why modern IDEs use color to tell you what something is: The brain can decode color and words in independent parts - unconsciously.

Packaging Convention

Next, he moves on how to split code into packages. Currently, we use a "package by layer" scheme, meaning all DB code goes into one package and the model code into another and UI layer in a third, etc. He proposes to use a "by-feature" packaging with the litmus test "you should be able to delete a feature by deleting a single directory, without leaving behind any cruft".

Uhm. When have you ever written any code where you could remove a feature just by deleting a class? This sounds nice and simple but it's fails Einstein's litmus test: "Make it as simple as possible but not more simple". Even if you have a plug-in based software like Eclipse, this doesn't work because there are still references outside (otherwise, your plug-in wouldn't be able to do anything).

Also, to keep a feature as isolated from everything else as possible (which is a good thing), you need to copy a lot of code into the feature which would otherwise reside elsewhere, neatly packed up in its own package. Really just a limitation of Java where you can't tell the compiler to generate boiler plate code for you. Still, you need to cut code in such a way that it reduces dependencies, not increases them. Therefore, a general rule won't cut (or maybe it will cut: you).

Immutables

John quotes: "'Classes should be immutable unless there is a very good reason for making them mutable,' says Bloch.". And later: "From a practical perspective, many widely-used frameworks require application programmers to use JavaBeans (or something similar) to model database records. This is deeply unfortunate, because it doesn't allow programmers to take advantage of the many positive qualities of immutable objects."

From a practical perspective, immutable objects are dead weight. Applications are all about changing something. I read data from the database, I modify it, I write it back. I rarely read, display and forget about something. Yes, immutables have advantages because they can be shared between threads but that's their only advantage.

Just think about this: You must modify data from the database. So you read the data into an immutable. How to modify it, now? Obviously, you need a method to change it. If you prefer setters, the "setter" must return a copy. So you need to copy the object for every single change. If you want to get a feeling for that, try to do math with BigDecimal. Okay, after the copy you can write the copy back to the database. Question: How do you notify everyone else who might have a (now stale) copy of the old immutable? There are no listeners; immutables can't have listeners. Duh. Driving this to the extreme, lists wouldn't offer methods to add or remove items; or rather they would return new copies of themselves after every add/remove operation.

Sorry, no sale. I can't add money to my cash register. It's immutable.

And a colleague just introduced me to another great concept: Constructors which require values for all fields. The class in question has 95 fields. This idea has the following flaws: a) No matter how big your screen, you can't fit the call onto it. b) After argument #10, you lost track and you can't see anymore which value goes into which argument. Now imagine you have to remove a field. How do you find the right one in this mega-call?

No, nothing beats the no-arg constructor plus a list of setters, all costs considered.

Private members

John proposes to move private members to the end of the class. Here, I agree. I'd even put them close to the getter and setter so that a lot of stuff that belongs together is together.

In todays IDEs with their superb code navigation (I can't really believe there was a time before the F3 key), this doesn't matter much, though.

Conclusion: Think about it, but don't bother.

No comments: