Improve the Smell of Your Code with Microrefactorings

You may believe that refactoring is about hideous complexity. Hours—no, days—spent trying to decipher thousand-line methods. A sprawling tangle of calls and callbacks that appear to produce working code only by accident.

Sometimes refactoring is all of these things, but more often than not, refactoring is about detecting familiar patterns and then responding to them methodically. From afar, large refactorings can seem daunting. Impossible. Close up, though, they’re made up of lots of tiny changes, and each change is rational. Explainable.

Refactoring is not so much about thousand-line methods. It’s about recognizing a tiny pattern within that thousand-line method and then applying a step-by-step recipe to change that small thing. It’s doing this over and over again.

Notice that there are two parts to this:

• Recognizing a snippet of code as exhibiting characteristics that are known to be problematic.
• Applying a change that is known to fix this category of problem.

This is not a rigid, infallible process. There’s some guess-work involved. Sometimes you recognize the problem, but it’s embedded in other problems that you have trouble articulating, and it can be hard to get at it. You might not be able to isolate it well enough to fix it. Or maybe you get a whiff of a problem but you can’t pinpoint what the issue actually is. Perhaps there are several known fixes for a problem, and you’re not sure which one will work and try two or three different approaches before you find one that fits comfortably.

Regardless, it all boils down to detecting a problematic pattern and then performing a systematic procedure to fix it.

Problematic patterns are known as “code smells”.

[A] code smell is a surface indication that usually corresponds to a deeper problem in the system. – Martin Fowler

A deeper problem in the system sounds kind of ominous. It feels like we’re right back to talking about the kind of thing that can take days to understand and weeks or months to fix. The thing is, the process is the same no matter how big—or small—the code smell is.

If you want to get better at refactoring, applying the process at the micro level can make it easier to start dealing with it in larger, more chaotic systems.

Look at the following code snippet:

sum = 0
numbers.each do |number|
  sum += number
end
sum

Make a snap judgement about it.

Good/bad. Clean/dirty. Simple/complex. Readable/cryptic.

You don’t need to be able to back up your gut feeling with logic or arguments. Just notice what you like or dislike about it.

Here’s another one:

anagrams = []
candidates.each do |candidate|
  if anagram_of?(subject, candidate)
    anagrams << candidate
  end
end
anagrams

Check your gut feeling about it. Compare it to the previous example.

Better/worse. Same/different.

Here’s another one:

mutations = 0
(0...strand1.length).each do |i|
  if strand1[i] != strand2[i]
    mutations += 1
  end
end
mutations

And one more:

oldest = ""
highest = 0
kids.each do |kid|
  if kid.age > highest
    oldest = kid.name
    highest = age
  end
end
oldest

Take a moment to notice similarities and differences between the examples.

The most obvious similarity is, perhaps, that these are all loops. Also, every loop is an each. More importantly, each of the above code examples exhibit one or two specific micro code smells.

• A loop with a temporary variable.
• A loop with a nested conditional.

The fix for both of these code smells is to choose a more appropriate enumerable method.

At first “choosing a more appropriate enumerable method” can seem like magic. How do you know? Well, you don’t. You basically go to the list of enumerable methods and see if you can find one that makes sense. You try a few. You start getting a feel for them. You find that in some cases more than one enumerable method does an admirable job. Over time you start reaching for good enumerable methods instinctively.

Here are the micro refactorings to match the micro smells showcased in the examples above.

The first loop has only a temporary variable, no nested conditional. It can be fixed with inject, or its alias, reduce:

# before
sum = 0
numbers.each do |number|
  sum += number
end
sum

# after
numbers.inject(:+)

The second loop has both a temporary variable and a nested conditional. We’re filtering a list, and both select and reject tend to be good for this sort of thing. In this case, since we’re filtering to keep rather than discard, select or its alias keep_if is a good choice:

# before
anagrams = []
candidates.each do |candidate|
  if anagram_of?(subject, candidate)
    anagrams << candidate
  end
end
anagrams

# after
candidates.select do |candidate|
  anagram_of?(subject, candidate)
end

The third example also exhibits both micro smells. In this case we’re counting differences:

# before
mutations = 0
(0...strand1.length).each do |i|
  if strand1[i] != strand2[i]
    mutations += 1
  end
end
mutations

# after
(0...strand1.length).count {|i| strand1[i] != strand2[i]}

The final code example has a triple whammy: two temporary variables and a nested conditional. Since we’re ranking on some attribute, perhaps sort_by would work:

# before
oldest = ""
highest = 0
kids.each do |kid|
  if kid.age > highest
    oldest = kid.name
    highest = age
  end
end
oldest

# after
kids.sort_by {|kid| kid.age}.last.name

It turns there’s an even better choice. If you want the first or last of some list of complex objects ordered by some arbitrary attribute, you can use min_by and max_by:

# even more after
kids.max_by {|kid| kid.age}.name

In the real world, the loop won’t always be an each. It won’t always be particularly verbose, either. Sometimes the conditional will be concise: a postfix if or unless, or a ternary statement.

Once you’ve identified a smell with this sort of precision, though, the problem is utterly straightforward to diagnose and fix in almost any context.

# before
words.inject([]) do |censured, word|
  censured << word if word.size == 4
  censured
end

# after
words.select {|word| word.size == 4}

Sometimes you won’t be able to get rid of it—there might not be a perfect enumerable method. This is the nature of code smells. They’re usually an indication that there’s a better way to solve something, but not always.

Unlike the classic code smells described by Martin Fowler in his book Refactoring, micro code smells are unlikely to be a symptom of a deeper problem in the system. A micro code smell will only affect a few lines of code in its immediate vicinity.

It’s still worth doing, and not just because you start getting a feel for how little drama there is in a good refactoring.

The cumulative effect of reducing friction, removing boilerplate, and improving readability in tiny increments is that larger code smells rise to the surface and become easier to detect.

And if you can detect it, there’s a great chance that you can fix it.