Improving on Sandi Metz's Gear Class from POODR

Chapter 2 of Sandi Metz's Practical Object-Oriented Design, An Agile Primer Using Ruby (POODR), "Designing Classes with a Single Responsibility", opens by giving an example of a class which represents the concept of a bicycle gear. First, a bit about the concept itself:

Gears work by changing how far the bicycle travels each time your feet complete one circle with the pedals. More specifically, your gear controls how many times the wheels rotate for each time the pedals rotate. In a small gear, your feet spin around several times to make the wheels rotate just once; in a big gear, each complete pedal rotation may cause the wheels to rotate multiple times.

Here's the code Sandi gives to introduce the way a gear ratio is calculated.

chainring = 52
cog = 11
ratio = chainring / cog.to_f
puts ratio

The "problem" with this code, from the perspective of what this book is trying to teach, is that it's not object-oriented. Ideally, we could package this behavior tidily into an abstraction that represents a concept in the application's domain. Here's what Sandi has to say about that.

The application [that's being designed in this chapter] will be made of Ruby classes, each representing some part of the domain. If you read through the preceding description [the excerpt above] looking for nouns that represent objects in the domain, you'll see words like bicycle and gear. These nouns represent the simplest candidates to be classes. Intuition says that bicycle should be a class, but nothing in the description lists any behavior for bicycle, so, as yet, it does not qualify. Gear, however, has chainrings, cogs, and ratios; that is, it has both data and behavior. It deserves to be a class.

That sounds reasonable enough to me. Here's Sandi's Gear class. Before you look, remember that this class represents not a gear is in a part of a machine, but a gear as in a gear ratio. The class could have been called GearRatio and thus been clearer in a sense, but then it wouldn't match the colloquial (and admittedly imprecise) language that cyclists use in real life. Think of gear to mean gear ratio the same way that acoustic can mean acoustic guitar or domestic can mean domestic beer.

class Gear
  attr_reader :chainring, :cog
  def initialize(chainring, cog)
    @chainring = chainring
    @cog = cog
  end

  def ratio
    chainring / cog.to_f
  end
end

puts Gear.new(52, 11).ratio
puts Gear.new(30, 27).ratio

When I first laid eyes on this code, my intuition told me something was off. What is it?

Call things what they are

Unfortunately, some of the code in the Gear class violates what is to me one of the most important principles in programming: call things what they are. At first glance I would expect chainring to be a chainring and cog to be a cog, wouldn't you? But chainring is not in fact a chainring, and cog is not a cog. Each of these variables is an integer whose value represents the number of teeth it has. In fact, Sandi spells this out for us in the text:

This Gear class is simplicity itself. You create a new Gear instance by providing the numbers of teeth for the chainring and cog.

This conceptual mismatch (chainring versus number of teeth in the chainring) results in a very awkward definition for the #ratio method:

def ratio
  chainring / cog.to_f
end

This code is nonsense, of course. You can't divide a chainring by a cog any more than you can divide, say, a dentist by a banana. (If you don't believe me, try it for yourself.) What we're really doing, of course, is dividing the number of teeth on the chainring by the number of teeth on the cog. Wouldn't it be clearer if we made the code match reality?

def ratio
  chainring_tooth_count / cog_tooth_count.to_f
end

Here's the full Gear class with that change in place.

class Gear
  attr_reader :chainring_tooth_count, :cog_tooth_count
  def initialize(chainring_tooth_count, cog_tooth_count)
    @chainring_tooth_count = chainring_tooth_count
    @cog_tooth_count = cog_tooth_count
  end

  def ratio
    chainring_tooth_count / cog_tooth_count.to_f
  end
end

puts Gear.new(52, 11).ratio
puts Gear.new(30, 27).ratio

I don't know about you, but when I look at that version of the class, my eye falls right on the #ratio definition, and it's abundantly clear to me what the method is all about.

The gear class from the outside

It's important to pay attention to the public API of this class, which indicates the class's understandability just as much as, if not more than, the class's actual content. To be clear, when I say "the public API of this class", what I'm referring to is this:

Gear.new(52, 11).ratio

What do 52 and 11 represent? From the outside, we have no way to know. Even if you happen to have laid eyes on the class's code quite recently (which of course you have!), you still have to exert a little mental energy in order to remember that the first parameter is chainring_tooth_count and the second is cog_tooth_count. Why make the reader do this when we can easily save them the trouble?

class Gear
  attr_reader :chainring_tooth_count, :cog_tooth_count
                                                                                                                                               
  def initialize(chainring_tooth_count:, cog_tooth_count:)                                                                                   
    @chainring_tooth_count = chainring_tooth_count
    @cog_tooth_count = cog_tooth_count
  end

  def ratio
    chainring_tooth_count / cog_tooth_count.to_f
  end
end

Now that the class takes keyword arguments instead of positional arguments, the class's public API looks like this:

Gear.new(chainring_tooth_count: 52, cog_tooth_count: 11).ratio

No more having to look up (or try to remember) what the arguments mean. If the arguments are labeled, they're impossible to misunderstand. Yes, the code is more "verbose", but what good is succinctness if it comes at the cost of understandability?

(To be fair, after I first wrote this post, I was leafing through my copy of POODR and I remembered that, in Chapter 3, Sandi actually does change Gear to take keyword arguments instead of positional arguments, so I can't judge the code in Chapter 2 too harshly.)

No speculative coding

Finally, we can improve this code still further by taking away a little bit of speculative code: code which is not driven by a concrete need, code that's added "just in case". What I'm referring to is the attr_reader which makes the chainring_tooth_count and cog_tooth_count attributes publicly readable. We don't currently have a need for these attributes to be publicly accessible, and doing so increases the size of the class's API, increasing the potential coupling between this class and its clients. The risk is low, but why have it at all?

class Gear
  def initialize(chainring_tooth_count:, cog_tooth_count:)                                                                                   
    @chainring_tooth_count = chainring_tooth_count
    @cog_tooth_count = cog_tooth_count
  end

  def ratio
    @chainring_tooth_count / @cog_tooth_count.to_f
  end
end

Based on the comments on a different post I wrote, my views toward this sort of usage of attr_reader are apparently controversial, so I'm not going to say that I think this last change is an objective improvement, just that I personally think it's an improvement. I don't think there's a good reason to write a piece of code until you're absolutely sure that you need it (and there's a test for it). Otherwise you're just gambling.

Setting aside the attr_reader change, let's review the other two improvements we made to this class. First, we made the #ratio method much more clear and coherent, merely by "calling things what they are":

# original version
def ratio
  chainring / cog.to_f
end

# new version
def ratio
  chainring_tooth_count / cog_tooth_count.to_f
end

Second, we changed the arguments of the Gear initializer from mysterious to clear by changing them from positional arguments to keyword arguments.

puts Gear.new(52, 11).ratio
puts Gear.new(chainring_tooth_count: 52, cog_tooth_count: 11).ratio

To me, these changes are both clear wins, and they serve to illustrate that just a couple minor tweaks can have a dramatic effect on the understandability of a piece of code. I believe these changes also show that as much excellent advice is contained in this great book which has helped so many people, POODR also contains some regrettably poor examples (such as the chainring and cog misnomers), and we shouldn't be afraid to point out these instances and improve upon them where we can.