This article is part 7 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.)
This is how we left things last time:
Coming back to this code after a couple of days away, I have the scary feeling that I’ve let a few problems build up. Before I continue, make a note of which violations of the 4 Rules you can see.
What I see (in no particular order) are these:
totalPriceOf
still has two parameters, and the caller can’t know which order they should be passed.By changing the price list keys to match the product names, I introduced those product names into the code of the function. It shouldn’t know about them.
The code and the test are coupled together by the two assumptions that prices are expressed as integers and product names/keys are expressed as strings. (This is Connascence of Convention, aka Connascence of Meaning; in code smell terms we might call it Primitive Obsession or Open Secret.)
I now have to make a decision: do I address any or all of these, or do I move on to add the next test? This is the kind of decision I — and all of us — make numerous times each day, often instinctively and with little or no conscious thought. Sometimes we’ll rush forward with the next test, sometimes we’ll fix whichever of those smells we’re attuned to on that particular day. Does it matter? Is there a “best” thing to do here? Or will we always converge on the same design eventually, regardless of which route we take?
A big part of me wants to move forward with the next test1. That's because I often like to allow the problem to get a little bigger before I clean up. As this code grows and evolves, the coupling I identified above will grow and evolve too, and often the solution patterns become easier to see when their "surface area" is larger. Or maybe I'm kidding myself here: perhaps I just simply enjoy solving bigger, hairier problems, and so I have a personal incentive to let the coupling get out of hand before tackling it.
Part of me also thinks that fixing these smells now could be gilding the lily: maybe I don't yet have "enough" code to be worrying about coupling -- after all, it's only 25 lines and I can see all the way across it from end to end. Surely I need more code to make the refactoring worthwhile?
Running counter to these thoughts, this series of articles is about refactoring, and so I am going to address those smells before moving on. Would I do this in “real life”? Let’s answer that case by case:
I feel that the two parameters to
totalPriceOf
play different roles in our story, because they change at different times. I can imagine — although I would need Customer confirmation of this — using the same price list to cost numerous baskets of items. And in fact the metaphor I unconsciously used in that previous sentence might suggest asking the price list to cost the basket2. Whichever way I choose to tackle this, I’m going to be altering the tested API. So I want to do that soon — partly because the code is small now and so there’s less to change, and partly so that I get early feedback on whatever design I choose. I’m going to fix this one now, to maximise my learning and avoid the potentially higher cost of doing it later.In order to eradicate knowledge of the product names from the code I suspect I will have to rewrite the algorithm. And with only the two tests I have currently, that feels like a risky enterprise. So I will leave this smell in the code until I have more tests — and maybe the addition of those tests will drive out those algorithm changes anyway.
In Javascript I don’t have static types, and so it’s really not obvious how best to cope with these assumptions about primitives. I guess I could wrap the integers, strings and arrays in objects of specific classes, and doing so would make those types explicit in the caller. Would that be too clunky, making the caller’s code less elegant and expressive? Well, given that I’m going to change the tested API anyway, maybe this would be a good thing to fix now too.
So I talked myself into immediately fixing two of the three problems I’ve identified, and while doing so I realised that there was one more example of Connascence of Convention that I hadn’t noticed earlier. Making lists turns out to be a valuable exercise yet again!
Does it seem as if I’m over-thinking all of this? What I’m trying to do is to unpick those micro-decisions we all make without much thought. Next time, I promise I’ll actually do some refactoring 😊.
Things to try
Before you add the next test, make an explicit list of the implicit coupling you can see in the area of code you’re working on. What should you fix before proceeding?
Think about how you decide whether to prioritise new functionality or refactoring. What kinds of coupling do you defer, and why?
As always, please leave a comment if you try any of these.
And finally…
Just a quick mention that I recently recorded an episode of the Mob Mentality podcast to talk about rhythm and cadence in both ensemble programming and TDD. The podcast is on YouTube:
Interesting metaphor there: “move forward”. Is that impatience? It’s been a couple of weeks since I added a new test, which feels like a long time. But if this exercise were playing out in real time only a couple of minutes would have elapsed.
I also note that I haven’t used the “basket” metaphor before. And in writing this I realise that the array of items is also a primitive, subject to the same Connascence of Convention coupling that I’ve noticed elsewhere.
Very interesting. I enjoy reading your thoughts and compare with mines to learn from that!
I agree that in real-life, I would also likely move on with other tests to "see" the refactoring becoming more obvious—at this step, I would be afraid of misunderstanding something and would need more "examples" to see if I get it right.
But this is a refactoring kata, thus it's a great exercise to try different things and see how it goes 👍