>Coding Style Cramps

>I recently rewatched “How to Design Good APIs and Why it Matters” (youtube video embedded below).

http://www.youtube-nocookie.com/v/aAb7hSCtvGw&hl=en_US&fs=1&border=1

There are many lessons that can be learned from designing a good api, one of the tips is to make sure that your method names make sense. I ran across some code today in a test case, named:

createStuff

Great, I used to write these types of method names when I was starting out. Figuring they were throw away, or just funny. However, over the years I tend to avoid these names. Why, because 6 months later I’m trying to remember what type of stuff is being created. The same thing goes with naming variables:

that

While it is cute, especially when you are comparing this to that, it can be harder to figure out 6 months later what that really entails depending on the code. Choosing proper names does help with the overall maintainability of the code. A computer may not care, but a human has to know quickly what is going on.

Another favourite I’ve seen lately is naming a variable soup. What is soup? What does it contain? Is it vegetable, egg drop, beef, or chicken noodle? Descriptive variables help yourself and those that have to maintain the code after you leave.

If you are not using a static analysis tool to help detect possible bugs ahead of time, why not? If you are looking for a way to help contribute to an open source project, but are not sure where to start, consider running FindBugs or PMD against that code base. Submit some patches to the committers. It is amazing the type of Homer Simpson “DOH!” moments these tools find.

An example:


if (someNullClass == null) {
_log.info("Error getting class name" + someNullClass.getName());
}

Obviously if this code ever was hit, it would toss a NullPointerException. However, I found similar code that had been in place for 3 years.

Your build system can help you out with reporting these types of issues. Hudson has the ability to display both FindBugs and PMD reports. These types of tools can really help clean up your existing code, and help to catch bugs before they can happen. I’ve also seen them help increase performance on some code bases. A very common pattern I keep seeing is string concatination in Loops. This is particularly nasty depending on the number of iterations, and is an easy thing to fix, by making sure you use StringBuilder or StringBuffer instead.

Here is a suggestion. Since for those that are on the Helios release train, M7 is supposed to be for bug fixes and documentation. Why not take a part of that time to run FindBugs or PMD against your code, and try to address as many of these bugs as possible. If you haven’t run these tools against your code base, you might be surprised what they find.

A constant gripe I hear from committers is that there is not enough resources to fix all the bugs, but how about we try to prevent some from ocurring in the first place. Then you can have more time to work on your features and that killer E4 application.

This entry was posted in clean code, craftsmanship, eclipse, refactoring, xml. Bookmark the permalink.

2 Responses to >Coding Style Cramps

  1. SteveL says:

    >yeah, the code should be if (someNullClass == null && _log.isInfoEnabled()) {_log.info("Error getting class name" + someNullClass.getName());}That way the NPE only kicks in if the log level is at info or debug, and if your log engine supports dynamic levels you can turn the NPE off without event restarting the app.

  2. David Carver says:

    >@SteveL: Personally, I'd avoid the NPE all together. The fact that you are trying to even access someNullClass's getName() method is probably wrong to begin with when the class is Null.The example code isn't exact, but it was probably cut and pasted from elsewhere.

Leave a comment