Refactoring Chapter 3— Bad Smells in Code
Deciding when to start refactoring — and when to stop — is just as important to refactoring as knowing how to operate the mechanics of it.
We (Martin Fowler and Kent Beck) have looked at lots of code, written for projects that span the gamut from wildly successful to nearly dead. In doing so, we have learned to look for certain structures in the code that suggest — sometimes, scream for — the possibility of refactoring.
One thing we won’t try to give you is precise criteria for when a refactoring is overdue. What we will do is give you indications that there is trouble that can be solved by a refactoring. You may not find the exact smell you can detect, but hopefully it should point you in the right direction.
Code Smells
MYSTERIOUS NAME
One of the most important parts of clear code is good names. Renaming is not just an exercise in changing names. When you can’t think of a good name for something, it’s often a sign of a deeper bad design case.
DUPLICATED CODE
If you see the same code structure in more than one place, you can be sure that your program will be better if you find a way to unify them. Duplication means that every time you read these copies, you need to read them carefully to see if there’s any difference. If you need to change the duplicated code, you have to find and catch each duplication.
LOOPS
We feel they are no more relevant today. We find that pipeline operations, such as filter and map, help us quickly see the elements that are included in the processing and what is done with them. Use and abuse of first-class functions.
MUTABLE DATA
Changes to data can often lead to unexpected consequences and tricky bugs. Functional programming is based on the notion that data should never change and that updating a data structure should always return a new copy of the structure with the change, leaving the old data pristine.
DIVERGENT CHANGE
Divergent change occurs when one module is often changed in different ways for different reasons. If you look at a module and say, “Well, I will have to change these three functions every time I get a new database; I have to change these four functions every time there is a new financial instrument,” this is an indication of divergent change.
LONG FUNCTION/ LARGE CLASS
All you have to do to shorten a function, 99% of the time, is to find parts of the function that seem to go nicely together and make a new one.
LONG PARAMETER LIST
If you can obtain one parameter by asking another parameter for it, you can replace the parameter with a query to remove the second parameter.
Rather than pulling lots of data out of an existing data structure, you can preserve the whole object to pass the original data structure instead. If several parameters always fit together, combine them introducing a parameter object.
SPECULATIVE GENERALITY
You get it when people say, “Oh, I think we’ll need the ability to do this kind of thing someday” and thus add all sorts of hooks and special cases to handle things that aren’t required. The result is often harder to understand and maintain.
Notes: For curiosity, Réaumur scale and Rankine scale links on wikipedia
CLUMPS
Bunches of data that hang around together really ought to find a home together. A good test is to consider deleting one of the data values. If you did this, would the others make any sense? If they don’t, it’s a sure sign that you have an object that’s dying to be born.
SHOTGUN SURGERY
Every time you make a change, you have to make a lot of little edits to a lot of different classes. When the changes are all over the place, they are hard to find, and it’s easy to miss an important change.
FEATURE ENVY
Feature Envy occurs when a function in one module spends more time communicating with functions or data inside another module than it does within its own module. We’ve lost count of the times we’ve seen a function invoking some getter methods on another object to calculate some value.
PRIMITIVE OBSESSION
We find many programmers are curiously reluctant to create their own fundamental types which are useful for their domain — such as money, coordinates, or ranges. We thus see calculations that treat monetary amounts as plain numbers, or calculations of physical quantities that ignore units
REPEATED SWITCHES (OR IFS)
We’ve even heard some people argue that all conditional logic should be replaced with polymorphism, tossing most ifs into the dustbin of history. The problem with such duplicate switches is that, whenever you add a clause, you have to find all the switches and update them. Against the dark forces of such repetition, polymorphism provides an elegant weapon for a more civilized codebase. The same applies for a lot of if/else clauses.
LAZY ELEMENT
Sometimes a structure isn’t needed. It may be a function that’s named the same as its body code reads, or a class that is essentially one simple function. This reflects a function that was expected to grow and be popular later, but never realized its dreams. Sometimes, it’s a class that used to pay its way, but has been downsized with refactoring.
TEMPORARY FIELD
Sometimes you see a class in which a field is set only in certain circumstances. Such code is difficult to understand, because you expect an object to need all of its fields. Trying to understand why a field is there when it doesn’t seem to be used can drive you nuts.
MESSAGE CHAINS
You see message chains when a client asks one object for another object, which the client then asks for yet another object, which the client then asks for yet another another object.
MIDDLE MAN
One of the prime features of objects is encapsulation — hiding internal details from the rest of the world. Encapsulation often comes with delegation. However, this can go too far. You look at a class’s interface and find half the methods are delegating to this other class.
INSIDER TRADING
Software people like strong walls between their modules and complain bitterly about how trading data around too much increases coupling. To make things work, some trade has to occur, but we need to reduce it to a minimum and keep it all above board. If modules have common interests, try to create a third module to keep that commonality in a well regulated vehicle.
COMMENTS
It’s surprising how often you look at thickly commented code and notice that the comments are there because the code is bad. A good time to use a comment is when you don’t know what to do. A comment can also explain why you did something.
When you feel the need to write a comment, first try to refactor the code so that any comment becomes superfluous.
