They say that if you put a frog into cold water and gradually bring it to the boil, the frog won’t notice. The same is true, I think, of Java devs (possibly others too, but for some reason it seems easy with Java). We get into hot water a lot, and it’s not because we jump in; it just heats up gradually around us. For instance:
- We make a domain object with a status
Status. - We update the status with a
StatusUpdate. - We define a service from which to retrieve the status updates. We call it
ServiceStatusUpdate. - We require the retrieval of updates to the status from the service with a
ServiceStatusUpdateRetriever. - We require a mapper to map the status update messages from the service to the status update objects. We call it
ServiceStatusUpdateRetrieverHelper. - We implement the mapper with a
ServiceStatusUpdateRetrieverHelperImpl.
If you’re thinking, “What’s wrong with that?” then write down ServiceStatusUpdateRetrieverHelperImpl on a piece of paper. Show it to everyone in the room. Someone will laugh. Ask them to explain the slightly manic, desperate edge to their laughter, and pair with them today.
Liz Keogh, lunivore 
Big warning signs there…
“Helper”: Any class with “Helper” in its name is badly named. “Helper” is so vague as to be meaningless. Help what? Look at the description of what the class does. Do the words “Helper” or “help” appear in the description? No. So it shouldn’t appear in the class name either.
“Impl”. It’s an abreviation. It’s barely pronouncable. It’s an abbreviation commonly needed when interfaces are being used incorrectly.
It also looks like you are using classes where methods might be more appropriate. If you need to update a status, why not have a status.update method?
I’m one of the people with a slightly manic, desperate edge to my laughter.
The status updates are XML messages coming from a number of 3rd party services. There is a status.update method which uses these. So it’s forgivable, at least while there are higher priority problems. I’d prefer an adapter on the service, perhaps.
The really odd thing is that almost everyone on this project knows better than to use Impl and Helper. Yet somehow they’ve crept in courtesy of the one or two who don’t know, or who do know it’s wrong but are scared to ask about the alternative. Those few have managed to start a trend that no one else wants to challenge, because most of the guys here are contractors on a client site, and still used to blame cultures and the need to cover one’s back.
A few of us got together and formed a posse to point out how ridiculous the class above was. It’s been renamed now.
Frogs. Hot water. Aren’t we all.
We’ve got a newly hired, very bright programmer who uses names like
notifyChatMessageReceived.
At first I hated the things, but now I’ve grown to love them. Despite absolutely no context, you can actually tell what this does. I’m getting even more extreme, calling methods things like tellAllOurListenersThatWeGotANewChatMessage
It’s of course impossible to edit this sort of code without an IDE, but then it’s not a Fortran compliant 6 character upper case only name either.
Of course “ServiceStatusUpdateRetrieverHelperImpl” is terrible, but not because it’s long. The problem is, it’s in Hungarian – Except for the ‘status’ core, everything is a decoration.
Further, it’s a highly decorated method name (uh, or is it a class, I’m lost here) that implements something so abstract it’s impossible for the uninitiated to understand what it all does. Try changing the variable names and see if you can make it not sound like code you’d dread debugging.
Why is Impl bad? Is the same true for a sub-class name like HttpServletRequest extends ServletRequest, or MyRedCar extends Car (I can’t think of a really silly example, but there are plenty I’m sure!)? any links to further reading on this? thanks 🙂
I too am intersted, can anyone answer this?