Last week I introduced the Open/Closed Principle, this week let’s see in in action. I’m going to warn you right off, this week’s code gets kind of ridiculous. It is the ne plus ultra of contrived examples.
And that’s ugly. No two ways about it. But this is a first draft and first drafts should be ugly. We just want to get up and running and pass the tests, which this does.
Is this code Open/Closed? That’s an impossible question to answer. Open/Closed isn’t a binary state, it all depends on what change we want to make. But Roman numbering hasn’t changed much recently, so maybe this code’s Open/Closed-ness isn’t all that relevant.
We are then surprised to hear that a new Roman empire has appeared. NeoRomans they call themselves. And, unlike their historic inspiration, they prefer round lines for their numbers. In their system:
- 1 = J
- 5 = R
- 10 = B
- 50 = P
- 100 = G
- 500 = Q
- 1000 = O
Is our code Open/Closed to implementing
to_neoroman? Let’s follow the steps from last week and find out. Those steps, again:
- Implement the new feature without changing any existing code
- Extract out the difference between your new code and the existing code
- Repeat as needed.
Step one, we implement
to_neoroman without changing any existing code. It gets pretty long, so I’ll just link to the commit.
Pretty terrible, right? I think we can agree that this is terrible. Before I had 4 duplicate methods, now I have 8. The new methods I introduced only differ in the letters they use, which gives us an obvious difference to extract out. Let’s go back to the previous commit, do the refactoring and try again. This time we’ll only change the design of the code. No functionality will be introduced or changed. And I do this by introducing a conversion parameter (Refactoring, p. 295).
And with that, the new Neo-Romans can be added thusly:
And now we can say that our code is Open/Closed to different lettering systems. We also spot some simple duplication between the
to_neoroman methods which we fix with Extract Method:
Surely no other Roman counting systems exist, so our code is as good as it needs to be. What’s that you say? We just discovered a new space-faring group of Romans that like to count up to 99,999? And they have new symbols for 5000 and 10,000? Since our code is Open/Closed now, we can surell just implement this new feature without changing any existing code!
Obviously our code is not Open/Closed to this change. Let’s follow the same process again. Now that we’ve tried implementing
to_exoroman without changing our existing code, we can refactor the code that made implementation such a pain, namely:
- The current
to_thousandsmethod only works up to 3,999
- We have no method for powers of ten greater than 103.
As has probably been clear from the first iteration, having all those methods for each power of ten is duplicating knowledge, and it makes the code hard to extend. If we fix that, we should be able to handle any power of ten.
In terms of the Refactoring book, this change most closely matches Parameterize Method (Refactoring, p. 283):
several methods do similar things but with different values contained in the method body. Create one method that uses a parameter for the different values
The code’s not great, but this change makes our code Open/Closed to the ExoRomans:
Whew. Space-faring Romans must be the last of the bunch. Oh, there’s a splinter faction of Romans that now want to count in multiples of 12? Well, ok, our code must now be flexible enough to handle anything…
This implementation shows that our existing code is only Open/Closed to multiples of 10. Back to refactoring. We extract out the 10-focused code into its own class, and then do another Extract Class to remove the conversion methods from Fixnum.
The full commit is getting long again. Here’s the full thing
With that implemented, we can add the
to_duodeciroman feature easily.
Set aside the quality of the code for now, I’m going to discuss that more in a second. Instead focus on what following the Open/Closed Principle has given us: Code that never stopped working. I showed this in detail last week, but it never hurts to mention it again. By never trying to simultaneously implement new features while re-designing my code, I kept my project in an always-working state. To me, that is the biggest win of following OCP.
Ok, now let’s focus on the quality of the code. Here’s the full final version. It’s not great, is it? That
convert method is super gross. Why, after following the super-great Open/Closed Principle, have we got such an ugly method? Mostly it’s because the features we implemented never challenged that method’s implementation. Our problems mostly stemmed from the parameters we passed to the method.
OCP by itself isn’t a comprehensive design tool. It’s a tool for applying grease to squeaky wheels. If a method in your code never needs to change, then OCP won’t ever make it change. Because of this if you follow only OCP then smelly code can hide and fester. Instead, you should always be on the lookout for other refactorings you can do after you finish implementing that new feature.
Speaking of smells, this week’s newsletter will start a series about the ‘official’ smells documented in Refactoring. Free to all comers, smelly or not. Sign-up is easy and you can always checkout previous newsletters. Comments/feedback/&c. welcome on twitter, GitHub or firstname.lastname@example.org