- From Code Smell to a New Class
- Dismantling the Conditional
- The Criteria You Accumulate
- Better Design, Better Tests
- Closing the Series
There’s a pattern that shows up in almost every project that has been in production for a while: a conditional that grows. It starts with a couple of branches, and every new requirement adds one more, because that’s the most obvious place to put it.
The problem is that this conditional makes extending the system possible only by modifying existing code. Every modification touches a line that already worked, so every new requirement arrives with its own chance of breaking something.
This article closes the series by showing where the design is headed: how to go from a conditional that grows to a set of objects that answer the same message. And above all, how to do it mechanically, in small steps.
Here’s the starting point:
class TimeAgo
def phrase(seconds)
case seconds
when 0 then "just now"
else "#{seconds} #{unit(seconds)} ago"
end
end
def unit(seconds)
if seconds == 1
"second"
else
"seconds"
end
end
end
And here’s the safety net, the same suite as in the two previous articles:
class TimeAgoTest < Minitest::Test
def test_phrase_for_many_seconds
assert_equal "47 seconds ago", TimeAgo.new.phrase(47)
end
def test_phrase_for_one_second
assert_equal "1 second ago", TimeAgo.new.phrase(1)
end
def test_phrase_for_zero
assert_equal "just now", TimeAgo.new.phrase(0)
end
end
And the requirement still pending is that durations stop being expressed only in seconds:
- 60 should read
"1 minute ago" - 90 should read
"1 minute and 30 seconds ago" - 3600 should read
"1 hour ago"
From Code Smell to a New Class
The previous article left a loose end. We named quantity, the concept of the amount being shown, but we couldn’t write it: for 0 seconds there was no reasonable value to return, and the code wasn’t open enough to hold it.
With the new requirement, 60 seconds has to show a 1, so the amount shown stops matching the seconds received, and quantity(seconds) would have to exist alongside the unit(seconds) we already have. Both would have to decide the scale, so both would grow a conditional of their own.
Let’s look at those two methods together, even though one still can’t be written:
- Both take the same argument.
- Both ask a question about that argument to decide what to return.
- Both are always used at once, interpolated into the same string.
That trio of signals is a classic code smell: Primitive Obsession. seconds is a bare Integer, and the meaning it carries in the domain (which scale it belongs to, whether it’s singular or plural) can be derived from it, but isn’t expressed anywhere. So every method that receives it derives that meaning again on its own, inspecting the value to work out which case it’s in. That repeated derivation is the clue that an object is missing.
Finding the Class
If several methods share an argument and their behavior depends on it, that argument wants to be the receiver of the messages, not a parameter. The question isn’t “what class do I create?” but “what is that number, really?” And it isn’t a number: it’s a duration.
Naming the class for the domain concept instead of for what it does right now is the same criterion we applied with unit. Duration will still be a good name once hours, days, or weeks show up.
The extraction uses the same tiny steps as always, leaving the tests green between each one. First the class with the state:
class Duration
attr_reader :seconds
def initialize(seconds)
@seconds = seconds
end
end
Then we copy unit inside, and in doing so its argument disappears: the object already knows its seconds.
class Duration
# ...
def unit
if seconds == 1
"second"
else
"seconds"
end
end
end
And we add quantity, the concept we named in the previous article but never got to implement:
class Duration
# ...
def quantity
seconds
end
end
Now TimeAgo asks the object for both pieces:
class TimeAgo
def phrase(seconds)
duration = Duration.new(seconds)
case seconds
when 0 then "just now"
else "#{duration.quantity} #{duration.unit} ago"
end
end
end
And now that nobody uses it, the original unit in TimeAgo can be deleted.
Notice what just happened with quantity. In the previous article, forcing it to return nil made the caller interrogate the result: a Liskov violation and the sign of a false abstraction.
What was missing was the object. A method can’t sign two contracts, and splitting it into two methods wouldn’t work either, because somebody would have to choose which one to call, and that somebody would be a conditional.
Dismantling the Conditional
With Duration in place, the case in TimeAgo is still there. And here we need to stop for a moment, because removing conditionals takes a criterion.
When a Conditional Is a Smell
A conditional is legitimate when it answers a question that doesn’t classify: a guard clause, a validation, a boundary, a business rule comparing two values. That kind of if expresses logic, and replacing it with objects wouldn’t help.
A conditional is a smell when it inspects a value to decide what kind of thing it is, and picks behavior from that classification. That conditional is doing by hand the work the message system already does. It’s type dispatch written out longhand.
Our case seconds is exactly that: it asks whether this duration is “nothing,” or “something on the scale of seconds,” or (any moment now) “something on the scale of minutes.” Because it classifies, it’s a smell.
One Branch, One Class
The process for turning the case into polymorphism is mechanical. I’ll call it dismantling the conditional, because we’re going to take it apart piece by piece without breaking anything:
- Isolate the conditional in a single place, if it’s still scattered.
- Turn each branch into the implementation of one same method, each in a different class. One of them can be a class you already have.
- Insert a creation point that picks the right class (a factory).
- Delete the original conditional.
Since the conditional is already isolated, we start at step 2. The else branch becomes the default behavior of Duration, and the zero branch becomes a subclass:
class Duration
# ...
def phrase
"#{quantity} #{unit} ago"
end
end
class ZeroDuration < Duration
def phrase
"just now"
end
end
Neither class contains a conditional about the scale. Each one knows a single thing, and both answer phrase. That’s the polymorphism: the message is the same, and whoever receives it decides what to do.
Step 3 is still missing, the point that decides which class to instantiate (the factory):
class Duration
def self.for(seconds)
if seconds.zero?
ZeroDuration.new(seconds)
else
new(seconds)
end
end
# ...
end
Now TimeAgo can start letting go of work. First, swap Duration.new for Duration.for:
def phrase(seconds)
duration = Duration.for(seconds)
case seconds
when 0 then "just now"
else "#{duration.quantity} #{duration.unit} ago"
end
end
Then the else branch stops composing the phrase and asks the object for it:
def phrase(seconds)
duration = Duration.for(seconds)
case seconds
when 0 then "just now"
else duration.phrase
end
end
Finally, the zero branch does the same:
def phrase(seconds)
duration = Duration.for(seconds)
case seconds
when 0 then duration.phrase
else duration.phrase
end
end
At this point the two branches are identical, so the case no longer decides anything and can be removed entirely. The duration variable goes with it, since it was only used once:
def phrase(seconds)
Duration.for(seconds).phrase
end
As you can see, the conditional hasn’t disappeared, it has moved. But it used to decide behavior, and now it decides a class, and it lives in a method whose whole responsibility is exactly that (the factory). We can call it a creation conditional.
Adding Minutes
Remember the two-phase discipline: first open the code, then add. Everything we’ve done so far was the first phase, and it hasn’t changed any of the program’s behavior. The code is open now, so we move on to the second.
These are the tests for the pending requirement, which you can add to the suite:
def test_phrase_for_one_minute
assert_equal "1 minute ago", TimeAgo.new.phrase(60)
end
def test_phrase_for_minutes_and_seconds
assert_equal "1 minute and 30 seconds ago", TimeAgo.new.phrase(90)
end
def test_phrase_for_one_hour
assert_equal "1 hour ago", TimeAgo.new.phrase(3600)
end
You can comment them out for now except the first one. If you run the suite, test_phrase_for_one_minute should fail.
The first step is a few adjustments to the base class (Duration) so it can support units other than seconds.
unit asks seconds == 1, but in Duration the amount shown always matches the seconds (since quantity simply returns seconds), so it can ask quantity == 1 without changing anything.
def unit
if quantity == 1
"second"
else
"seconds"
end
end
Green. This move replaces a concrete value with the reference that generalizes it, with the extra effect that unit stops looking at the seconds received and starts looking at the amount shown, which is what each scale is going to redefine.
The next adjustment is naming the word that varies. “second” and “seconds” share a root, and that root is what each scale will have to supply. We’ll call it scale, with the base class value (even though nobody calls it yet):
def scale
"second"
end
Green. Now each branch has to adopt it separately: first the singular starts returning scale, and then the plural becomes "#{scale}s". The base class ends up like this:
class Duration
# ...
def unit
if quantity == 1
scale
else
"#{scale}s"
end
end
end
Now we can add the minutes scale with a class of its own:
class MinutesDuration < Duration
def quantity
seconds / 60
end
def scale
"minute"
end
end
We add the new class to the factory:
class Duration
def self.for(seconds)
if seconds.zero?
ZeroDuration.new(seconds)
elsif seconds >= 60 && seconds < 3600
MinutesDuration.new(seconds)
else
new(seconds)
end
end
# ...
end
The tests pass now.
Let’s refactor the factory’s conditional into a case:
class Duration
def self.for(seconds)
case seconds
when 0 then ZeroDuration
when 60...3600 then MinutesDuration
else Duration
end.new(seconds)
end
end
The tests stay green.
Uncomment the next test, test_phrase_for_minutes_and_seconds, which should be red. This is the compound case: 90 seconds is one minute and thirty seconds.
The temptation is to put a conditional inside MinutesDuration asking whether there’s a remainder. But look at what it would be asking: whether the remainder is “nothing” or “something.” That’s the same classification we just dismantled, and we already have a class for the “nothing” case.
Instead of a conditional, we add a new parts method with an array, to break the phrase into pieces:
class Duration
# ...
def phrase
"#{parts.join(" and ")} ago"
end
def parts
["#{quantity} #{unit}"]
end
end
Now we can override parts in the subclasses:
class ZeroDuration < Duration
# ...
def parts
[]
end
end
class MinutesDuration < Duration
# ...
def parts
super + remainder.parts
end
def remainder
Duration.for(seconds % 60)
end
end
The test should be green now.
Uncomment the last test, test_phrase_for_one_hour, which will be red, since it returns "3600 seconds ago" instead of "1 hour ago".
Add a new class:
class HoursDuration < Duration
def quantity
seconds / 3600
end
def scale
"hour"
end
end
And add it to the factory:
class Duration
def self.for(seconds)
case seconds
when 0 then ZeroDuration
when 60...3600 then MinutesDuration
when 3600.. then HoursDuration
else Duration
end.new(seconds)
end
# ...
end
The suite is now fully green.
Let’s check how the result turned out:
- For 60 seconds,
MinutesDurationreturns["1 minute"]plus the pieces of aZeroDuration, which are none:"1 minute ago". - For 90,
["1 minute", "30 seconds"]:"1 minute and 30 seconds ago". - For 61,
["1 minute", "1 second"]:"1 minute and 1 second ago", with the singular correct and without anyone writing a rule for it. In the previous article, the naive version of this requirement needed an's' if seconds > 61buried inside an interpolation.
We’ve just seen the Open/Closed principle in action, and the mechanism that makes it possible is polymorphism. Duck typing would be an alternative, but inheritance fits here because the variants are genuinely specializations of the same thing, and they share almost the entire template.
The Criteria You Accumulate
By the end of a series like this one, what you’re left with isn’t the code, it’s a set of criteria that give you the next step in a moment of uncertainty. We’ve been piling them up along the way:
- Wait for a requirement before designing
- Tolerate duplication rather than commit to a false abstraction
- Remove one code smell at a time
- Make one-line changes and run the tests
- Don’t invent requirements
None of them requires prior experience with the problem. Only discipline.
But there are two more that deserve names of their own.
The first is write the code you wish you had. When you don’t know how to implement something, write the call you’d like to be able to make, as if the object answering it already existed, and only then implement it. That’s what happened when phrase called parts before it existed, because that was exactly what phrase needed to ask for. Designing this way forces you to think about the message first and the object that serves it second, which is the right order (and often not the usual one). There’s more in Messages Before Objects.
The second is ask for the result, not the pieces. Compare these two versions of TimeAgo:
"#{duration.quantity} #{duration.unit} ago" # before
duration.phrase # after
In the first, TimeAgo knows the anatomy of the phrase: it knows there’s an amount, a unit, and a suffix, and it knows what order they go in. That knowledge is a dependency, and any change to the shape of the phrase reaches it. In the second, TimeAgo asks for what it wants and lets the object decide how it gets built.
When to Improve Code Voluntarily
Refactoring with no requirement to justify it carries a real opportunity cost, and there’s ugly code that has worked for years without anyone touching it: that code doesn’t need to be changeable, because it doesn’t change.
Two reasonable criteria for doing it anyway:
- When the code is in your way today, and not in some hypothetical future.
- When you understand it better now than whoever touches it after you will.
In either case, decide up front how much time you’ll spend on it. This kind of improvement has no natural end: there’s always something else to clean up. When the time runs out, stop and look at what you have. If it improves on what was there, keep it. If you left a concept half-named, undo back to the last stable point. Tiny steps are what give you that freedom.
Better Design, Better Tests
The design we’ve just built changes what you can test, and how.
The first thing to note is something that has already happened: the tests from the beginning are exactly as they were when we started. We’ve gone from one class with a case inside it to five classes and a factory, and those tests haven’t changed. That’s the consequence of describing behavior instead of structure.
But they’ve also changed in nature. When we wrote them, phrase was a method with a case inside, so they tested a unit. Now a single assert on phrase(90) runs through TimeAgo, the factory, MinutesDuration, and the Duration of the remainder. They’re still valuable, because they verify that the pieces fit together, but they’re no longer unit tests: they’re integration tests. And that has a price, which is diagnosis: when one turns red, it will tell you the phrase came out wrong, not who spoiled it.
The advantage we have now is that each variant is a class you can create and question directly, so those unit tests should be added.
Here’s an example of a unit test for ZeroDuration:
class ZeroDurationTest < Minitest::Test
def test_stands_alone_as_a_phrase
assert_equal "just now", ZeroDuration.new(0).phrase
end
def test_contributes_nothing_to_a_larger_phrase
assert_equal [], ZeroDuration.new(0).parts
end
end
The second test is the one that makes 60 seconds read "1 minute ago" and not "1 minute and 0 seconds ago".
Keep in mind that what’s being tested isn’t the class but the role: something that knows how to turn itself into a phrase and into pieces of a phrase. Classes are an implementation detail and can change; the role is the contract, and it’s what should survive the next refactoring. There’s more in Tests That Survive Change.
Closing the Series
Let’s look back over the whole path:
- In the first article we started with the simplest code that passed the tests, without applying any design.
- In the second article a new requirement arrived, and we applied the convergence rules to let an abstraction we hadn’t foreseen emerge.
- In this third article we opened the code for extension, and that let us dismantle the conditional, turning each branch into an object. The system is now open to scales that don’t exist yet.
We never needed to see the destination, the design, from the start. That’s the advantage of what we’ve covered: the most appropriate design emerges from a procedure you can follow in small steps, with the tests as a safety net, when you don’t know what to do.
Test your knowledge
-
When is a conditional a code smell?
-
After replacing the conditional with polymorphism, the factory still contains a
case. What did we actually gain?
-
quantityfailed in the previous article because it had to returnnilfor zero. Why does it work now?
-
What does "coding by wishful thinking" mean in practice?
-
The three original tests never changed while the code went from one class to five. What happened to them?

Comments
Nothing yet. Say the first thing.
Sign in to join the conversation.