How Many Lines Should be in a Class?

If there’s one thing I know, it’s that developers love to argue. It can be the important things like naming and design all the way down to silly things like where a curly braces should go¹. One thing we all can agree on is that software is easier when it’s broken down into small chunks. We just don’t agree on what small is.

How many lines should a class be? This Clean Code cheat sheet says classes should be less than 100 lines, but it doesn’t specify language or platform. Now take a look at this thread where people say things like:

Putting a number on the amount of lines a class should have is something I disagree with. – BeachBum09

In some classes I’ll spend at least 100 lines sanity checking responses – Kinglink

This is the kind of dogmatic, out-of-nowhere rule that’s always bothered me from the XP crowd. – alextk

I could go on, but it’s clear that we can’t agree on a number of lines that makes a class too big, and I’m inclined to agree. Indeed how do you even count a line, do header files count? Curly braces? And if so, does that mean a class in Ruby can have far more functionality than one in C++ or Objective-C? Should every String, List and Hash implementation be rewritten to be under 100 lines? That’s gonna take a while.

You’re measuring the wrong thing

The real reason we can’t agree on a number of lines is that there is no right number. It’s not lines that matters, it’s cohesion. To borrow from good old wikipedia - cohesion refers to the degree to which the elements of a module belong together. We also refer to this as the Single Responsibility Principle, but the SRP is more easily identified when a module changes. When we have to change a class for more than one reason we break it up into multiple classes. How do we identify a class that has poor cohesion before it becomes a problem?

Look to the Tests

When I started this I was going to tell you to look into LCOM4 as a guideline, and you should probably still read that but the truth is there’s a simpler way to tell and you can look to the tests. I’ll use Ruby for this example (borrowed partially from RSpec Rails docs):

RSpec.describe TeamsController do
  describe "GET index" do
    it "assigns @teams" do
      team = Team.create
      get :index
        expect(assigns(:teams)).to eq([team])
    end

    it "renders the index template" do
      get :index
      expect(response).to render_template("index")
    end
  end

  describe "POST team" do
    it "creates a team" do
      post :team

      expect(Team.count).to eq(1)
    end
  end
end

Nested describe/context blocks, or multiple test classes, for the same class are a sign. I won’t say smell, because there are many legitimate reasons to have more than one context for testing a class, but they are a certain hint. Note here how we have two descriptions of a class but they share nothing. There is no data being changed in each one, indeed the POST request doesn’t need to look at the @teams instance variable. This is because Rails controllers typically aren’t cohesive. POST and GET are only related by web urls, and don’t share data. Indeed a Rails controller won’t have state at all by definition. Each request is new.

So Rails controllers are not particularly cohesive, and we can see that when we look at a spec, but what does that tell us about code generally? Well when you have to use many contexts for a test class it’s a sign that the class itself has more than one responsibility. Each context could likely be it’s own class, and it’s a far better indicator than how many lines you’ve written.

Solution

So now that we can see a class isn’t cohesive, what is the solution? You have two choices. If the system depends on that interface, like in a Rails Controller, you can refactor that class to the Facade pattern. Each action could use a cohesive class, typically the Command pattern, thereby limiting the number of classes affected by change to the controller and eliminating noise and confusion in the controller itself. If the entire system doesn’t depend on that interface, and those objects are grouped together merely by coincidence, then it’s time to do one or more Extract Class refactorings to clean it up. Usually you won’t want to do this all at once, instead extract each class individually at the time you need it. This way you can balance the need for features now with the need for maintainable code in the future.