Smell A Smell Episode 2: Large Class

Written by TJ Stankus

At UserTesting, refactoring code is a way of life. In this second installment of "Smell a Smell," learn the warning signs of large classes.

While code smells have names ranging from the creative (Shotgun Surgery) to the criminal (Indecent Exposure), Large Class is what it is. Toiling Class or Slogging Class would also be appropriate because this code smell indicates not just that a class is too large, but that it’s working too hard. In the Ruby on Rails world, the term “fat” is often used, a la Fat Controller or Fat Model.

So, what are some of the symptoms that indicate a class is indeed too large?

Symptom: Too many lines of code

One obvious indicator is that there are simply too many lines of code in a class. But how many is too many? While opinions differ, one simple and useful metric is the class size guideline in Sandi Metz’s Rules for Developers, which states that a class should be no more than 100 lines of code.

In Ruby on Rails applications, it’s common to see controllers and models that are (much) larger than 100 lines of code. Our customers and applications are better served by thoughtful designs which lead to reasonably sized classes. In a recent podcast, the creator of Rails, David Heinemeier Hansson (DHH), states that he keeps controllers small by restricting them to the default REST actions. The book Growing Rails Applications in Practice advocates reducing every user interaction to a Rails CRUD resource with its own controller, even when there is not an exactly correlated model. For example, a screen to upload multiple images at once might post to an ImageBatchesController even though there is no ImageBatch model.

For ideas on decomposing large Rails models, the classic Code Climate blog post 7 Patterns to Refactor Fat ActiveRecord Models provides a number of ideas for breaking off parts of a large model into more manageable, testable, decoupled units.

Symptom: Too much internal state

Another indicator that a class might be too big is that it has a lot of internal state. This state may take the form of instance variables or attribute readers. When faced with more than a few bits of internal state, it’s instructive to step back and assess how that state might be better organized. Perhaps several bits of state are logically grouped together and represent a data clump that can be the starting point for an extracted object? Perhaps one bit of state makes sense in an immutable value object?

Symptom: Constructor takes too many arguments

A constructor (in Ruby, a class’s initialize method) that takes too many arguments is on the fast-track to becoming bloated. It leads to classes with too much internal state. Referring to Sandi Metz’s Rules for Developers once again, she states that methods should receive no more than four parameters and that hash options and keyword arguments each count as a parameter. A constructor is a method like any other. I’d go even one step further and reduce the guideline to three parameters. Your design sensibilities should be tingling if your constructor takes more than three parameters. As with any rule, if you feel the need to break it, you should get buy-in from your coworkers.

Symptom: Repeated prefixes and suffixes

Sometimes opportunities for abstractions are staring you right in the face in the form of names chosen for methods, instance variables, and the like. It’s common to see names that share a common prefix or suffix. For example, an AccountTransaction class might have methods to handle deposit_amount and deposit_currency. It stands to reason that Deposit would represent its own class abstraction and have amount and currency as its internal data. Identifying repetitive prefixes and suffixes and extracting those concepts into their own class abstractions leads to simpler, more decoupled code.

Symptom: Misplaced behavior

One of the more confounding object-oriented design principles is the Single Responsibility Principle. It’s easy to say that a class should have one responsibility, or even one reason to change. But followed to its logical conclusion, this leads to classes with a single method and a single behavior. While constraints often engender better designs, the Single Responsibility Principle, taken at face value, feels too constrictive. I tend to think of this OO principle as the Focused Responsibility Principle and enforce its value by asking questions of my classes. For example, given the following code from a popular code kata, one might ask, “BowlingGame, are you a strike?”

class BowlingGame
  # ...

  def strike?(frame_index)
    # ...
  end

  def spare?(frame_index)
    # ...
  end
end

Of course, this question makes little sense. We know that “strikeness” does not apply to a game, but to a frame, so it makes sense to move this behavior to a Frame abstraction. The question, “Frame, are you a strike?” sounds much better. Combined with the guidelines around class size and sensible method names, demanding that your classes justify ownership of their behaviors is another way to keep your code lean and maintainable.

Curative refactoring

Now that we’ve identified a number of indicators for a class that’s too large, what do we do about it? The wonderful thing about code smells is that the literature provides explicit guidance. Martin Fowler’s classic Refactoring: Improving the Design of Existing Code book (and its updated Ruby Edition) lists Extract Class, Extract Subclass, Extract Interface, and Replace Data Value With Object as curative refactorings for the Large Class code smell. For each of these code cures, Refactoring additionally lists specific steps for transitioning code from one shape to another. These mechanics, if followed closely, enable you to maintain green tests at every step along the way. Many programming texts do not actually contain detailed, nitty-gritty code advice, but Refactoring contains it in spades. A careful reading will improve your day-to-day coding life.