Javascript Checkout 4
In which we consider self-similarity, and a branch is created!
This article is part 4 in a series in which I’m doing a well-known code kata in the “TDD as if you meant it” style. I’m also strictly following the 4 rules of simple design and looking for opportunities to use implicit coupling to drive refactoring under the Once And Only Once rule. (If you missed the start of this series you can catch up with part 1 here.)
Last time I spent quite a bit of effort getting my code and tests readable (in the sense that they tell my Customer’s story). I arrived at a point where I felt it was time to introduce the next failing test, but I promised to review the code one last time before making that big step.
So here's where we are right now:
Coming back to this now, I can see that there’s still some coupling that I should address.
Firstly, zero appears three times (lines 2, 3, and 11). The instance on line 3 is clearly unique, but those other two “mean” the same: they represent the price of an empty basket. In past attempts at this kata I’ve never bothered about that, but I can definitely conceive of situations in which there could be a non-zero fee for engaging the purchasing process — perhaps a membership fee or reservation fee. But looking ahead at Pragmatic Dave’s tests I can’t see any mention of such a thing for this kata, so I’m happy to let those zeroes stand as they are.
Am I “allowed” to look ahead? I think so — I suspect that’s what my Customer is doing much of the time. But what about YAGNI (You Ain’t Gonna Need It), I hear you cry? Well, I’m not deciding whether to add functionality; only how much to refactor. In terms of implicit coupling, I guess I’m deciding that it’s okay for the zero starting price to be implicit in this codebase, and that adding a constant or suchlike would be overkill based on what we know now. I don’t want to pre-load this code with every future I can possibly think of, and so I’m happy for now to assume that what we can see in the future tests is all we’ll have to cope with.
Next though, there are 50s on lines 4 and 12. This feels like an arbitrary value, and the kata description ends with this paragraph:
“The challenge description doesn’t mention the format of the pricing rules. How can these be specified in such a way that the checkout doesn’t know about particular items and their pricing strategies? How can we make the design flexible enough so that we can add new styles of pricing rule in the future?”
So my instincts are borne out by the Customer: the 50 is an arbitrary value and represents unwanted coupling between the code and the test. And I’d say that the coupling is implicit, because I see nothing in the totalPriceOf function that tells me how the 50 is likely to change.
The question is, what to do about it? As I’ve mentioned elsewhere, some folks would triangulate here, adding another failing test that demonstrates the coupling. Indeed many times in the past I’ve added a new test
at this point. Pragmatic Dave takes a slightly different slant on the same idea:
Me, I’m always nervous about triangulation. I don’t like the thought of adding more complexity to code that is already coupled — it seems like an added maintenance burden and we still need to fix that coupling.
But that’s a purely subjective view, so let’s put it to the test. At this point I’m going to branch: I’ll triangulate on one branch and then come back to trunk and fix the coupling instead of triangulating. Then we can compare the two approaches and see what benefits each gave us.
So I create a branch triangulate-first and add Pragmatic Dave’s next test. It fails, so I quickly make it pass:
Commit: calculate price for basket with A and B.
There are, of course, plenty of ways I could have changed the code to make this test pass, so why did I choose this one? I don’t honestly know. I also considered something like this:
Both variants have the advantage that they are pure additions, leaving the code around them unchanged. And arguably the second variant is a direct, naive interpretation of the new test, whereas in order to get to the first variant I must have done subtraction in my head. So in some sense the “distance” from the new test to the second variant is shorter, and maybe that’s a better habit to have.
That said, the option I chose does reflect the pricing rules from the original kata description (the price of a B is 30). More importantly though, it also extends the structural pattern of the original code. I think I have a strong personal preference for designs that are expressed through symmetry and self-similarity. This may also be why I like my test cases organised in a table. Nevertheless, I took a design step while getting from RED to GREEN, and in general I try to caution against doing that.
Anyway, we are where we are, so let’s stick with it and see where it takes us. It’s time to refactor. Following the 4 Rules, I need to start with readability. And if I want to tell a story I should begin at the outside — in this case that means with the tests. I didn’t change any names, but I did violate the layout we introduced to help deal with all those brackets. So I change lines 13-14 to return to that standard:
This is probably going to become tiresome, so I may need to think of something better if it does. Commit: ensure consistent layout of test cases.
I feel those tests are (still) pretty readable, so let’s look at the code. If I forget everything I know about future requirements (ie. the remaining tests in the kata description), then the code as it stands seems reasonably clear. I can’t think of a way to improve its story-telling ability.
So I feel it’s time to move on to rule 3 — does this codebase say everything once and only once? We’ll find out next time…
Things to try
Take a break after any period of concentrated effort. When you return you’re likely to see things with fresh eyes.
Next time you’re getting from RED to GREEN jot down as many alternative code changes as you can think of. Which one do you choose to move forward with, and why?
Look for opportunities to increase the symmetry and self-similarity in your code. Does this always help with readability?
As always, why not let your fellow readers know how you get on when you try these…