Some Code Smells
- Duplicated Code
- Duplicated code is a recipe for disaster, as
it's almost always the case that during maintenance the two fragments will
start to diverge. Extract a method with the common code and invoke it from
the various duplication locations.
- Long Method
- Long methods put too much of a burden on short
term memory. Try to extract sections of code as private methods and call
these methods. A decent heuristic is to look for sections with a header
comment - the comment may suggest an appropriate method name.
- Large Class
- Large classes are almost always in indication
of low cohesion (and possibly high coupling). Try refactoring into two or
more independent classes.
- Long Parameter Lists
- This is often an indication that the wrong
object has been assigned some responsibility - probably one of the objects
in the parameter list. Alternatively, the parameters may represent a
coherent concept that should be encapsulated in a new class.
- Switch Statements
- Switch statements are often indications of the
use of a "type code" to determine what processing should be done.
Consider the use of polymorphism to dispatch to the appropriate algorithm.
If the switch statement is only a piece of a larger method, try to extract
the switch statement into its own method which will then be a Template
Method (GOF) in an abstract super-class.
- Speculative Generality
- If a method solves a more general problem than
is necessary, and if as a result the method becomes long and/or complex,
consider backing off to an algorithm that solves the actual problem at hand.
Of course the use of generality to create a more general and simpler
algorithm is encouraged.
- Temporary Field
- An instance variable that is assigned a value
before it is referenced in all methods is really a
temporary variable in disguise. Remove the instance variable and insert a local variable in those methods that use it.
- Message Chains
- Long chains of method calls to values returned
by other method calls couple and object to the navigation structure
currently supported by its direct connections. Try shielding the current
object from such navigation by providing helper methods in the direct
connection objects. By the way, this is essentially a restatement of the Law
of Demeter.
- Data Class
- A data class is one supporting only simple
accessor (getter) and mutator (setter) methods. This is often an indication
that some other classes (or possibly a single class) is the focal point for
all the interesting behavior in the system. Look to refactoring by moving
responsibilities out of behavior classes to data classes.
These are just a few of the code smells you should avoid. For more on code smells, visit http://c2.com/cgi/wiki?CodeSmell and/or read Refactoring: Improving the Design of Existing Code by Martin Fowler, Addison-Wesley, 1999.
Revision: $Revision: 1.4 $ $Date: 2007-10-22 02:05:50 $