During my work with various clients and projects on Java, I repeatedly run into smelly code, and find that my now ancient presentation from year 2000 is still as relevant as ever. The presentation I am talking about is on Bad Smells in Code that was derived from Martin Fowler’s book Refactoring: Improving the Design of Existing Code. While the Refactoring book itself was a little too prescription oriented for my liking, the chapter about code smells was very good in my opinion. I look upon code smells as fairly common classic mistakes made in developing code that you are best advised to avoid – they are usually indications that something is not right in paradise and needs a closer look, and possibly some refactoring (i.e. clean-up) is in order. This blog entry is a rehash of one from my personal blog – http://vivekagarwal.wordpress.com/2008/06/07/dusting-off-the-bad-smells-in-code-presentation; reason for regurgitating it is the relevance of this subject and its near-and-dear-to-my-heart nature!:)
What is a code smell?
A code smell identifies classic mistakes made in developing code. These mistakes typically result in code that is difficult to understand, maintain, debug, and extend.
What are some common code smells in object-oriented code?
- The worst stinker of all!
- Difficult to reuse and difficult to understand.
- The object programs that live best and longest are those with short methods.
- The best object programs are endless sequences of delegation. Their payoffs are explanation and sharing.
Long parameter list
- Hard to understand, methods become inconsistent and difficult to use and more likely to require change.
- Does the name of the method succinctly describe what that method does?
- Could you read the method’s name to another developer and have them explain to you what it does? If not, rename it or rewrite it.
- Pick a set of standard terminology and stick to it throughout your methods.
- If you use “add”, then use “add” in all relevant situations – not “create”. Another example is “delete”/”remove”.
- Classes delving into each other’s private parts.
- Excessive use of class A’s accessors from class B.
- Over intimate classes need to be broken up as lovers were in ancient days! ?
- Beware of classes that unnecessarily expose their internals. You should have a compelling reason for every item you make public. If you don’t, hide it.
- Too much code
- Too many instance variables
- Use layering and decomposition to avoid this problem.
Classes with too little code ?
- Often data classes that can become domain classes
- Or candidates for downsizing!
- A class is commonly changed in different ways for different reasons
- Example – database + functionality
A large number of function classes
- Indicate that the designers are thinking upside-down, i.e.. thinking of that which can be done to an object rather than that which can done by the object.
- A method that seems more interested in a class other than the one it actually is in.
- Many messages to the same object from the same method
- Data items often hang around in groups together.
- Extracting them into a class of their own will help reduce field lists and parameter lists.
- Leads to duplication which leads to shotgun surgery
- Consider polymorphism instead.
- Planning for change that never occurs
- Increased complexity
Too many casts
- Over generalization
- Often used as a deodorant for stinkers
- Superfluous comments
There are other smells too but these are the most important ones in my opinion.
To summarize, code smells give you an indication that there is trouble in paradise. They are only a guideline and cannot be used as absolute directives. When you realize that you are writing code that reeks of one of these smells, then STOP and ask yourself if you are proceeding down the right path. If not, REFACTOR.
Also make the time to look at How to Design a Good API & Why it Matters – this presentation by Joshua Bloch is relevant even if you are not into designing APIs and all you do is design/code! This is part of the orientation process for all new Java developers on-ramping to my team!