This post is part of an ongoing series about Unit Testing.
In my last post, I talked about testing pure functions using the Coin Changer code kata. Here are the tests we ended up with:
Handling Different Sets of Coin Denominations
The one feature we haven’t tested yet is the ability to pass in a different set of coin denominations. There are a couple of ways we could do that:
Pass the coin denominations directly into the
exchangemethod. This would allow
exchangeto remain a pure function that is unaffected by anything outside of itself. However, every caller must then know the set of coin denominations.
CoinChangeron the list of coin denominations and then use them from within the
exchangemethod. This means that callers only need a
CoinChangerand don’t need to know the particular coin denominations. However, we would need to create a new
CoinChangerevery time we wanted to use a new set of coin denominations.
If we were using this kata for practice, we’d try each of the options
in turn and see how the design works out either way. In real-world
code, we’d look at how the rest of the application is likely to use
CoinChanger and make a decision based on the wider context.
If we choose option #1, the tests are relatively easy. We just add a
coins column to our test table above and add new rows to cover the
cases of interest.
Option #2 introduces some interesting issues, so let’s go that direction.
I’m pretty confident that the code we’ve written so far will handle other sets of denominations that have similar properties to the USD coins we’ve been using so far. However, there are two special cases I can think of:
Canada has eliminated the penny from circulation; thus, we need to round to the nearest nickel when calculating change.
There are possible sets of coin denominations that will cause our implementation to fail if we’re not careful. For example, if we have coin denominations of 10, 7, and 1 and we want to make change for 14, we should return two 7-cent coins, not a 10 and four 1’s. Remember that we are supposed to return the fewest number of coins. A typical algorithm will return too many coins.
Here are the tests we might end up with after handling these cases:
These specs are quite reasonable, and many people would suggest leaving them as-is. But there are two bits of duplication and a missed opportunity for communication that I’d like to consider.
Extracting the Test Blocks
Notice that each of the three
describe blocks is structured exactly
the same. There’s an array of test cases followed by a block that
it blocks for each test case. The
it blocks are nearly
identical. In the middle describe block, the description is slightly
different than in the other two cases.
There are a couple of ways we could eliminate the duplicated
blocks. First, we could extract a method and pass in the array of
Second, we could extract the test block into a method returning a lambda:
Both of these work and both eliminate the duplication. Notice that
we lost the somewhat customized messages for the middle
block. Given that the key concept (rounding) is communicated by the
describe, that’s not too big a loss and I’m willing to live with it.
It is unfortunate that both methods require the
self. prefix but,
since we’re executing the methods outside the context of an
block, we need to do that. Similarly, the methods have to be at the
top of the spec so that they’re available when RSpec evaluates the
calls to the methods. I’d have preferred to hide them away at the
Overall, I prefer the second approach (extracting the test block)
because it keeps the test case table front-and-center in each test.
That’s where I want the reader’s focus; the test block shows up as
almost an afterthought, and that’s exactly what I want. In the first
approach, the reader encounters the
generate_specs call first and
has to start thinking about that while trying to process the test case
By extracting the duplication, we’ve had to get a little more clever with the code. This can be a disadvantage. In this case, I think it makes the tests more readable. I don’t have to study the three duplicate test blocks to see if there’s anything special about any of them; I know they’re identical. I have one place to look to see details about how the test cases are executed, and one place to change if I later refactor the code.
All in all, I think this extraction is a net win.
Extracting the Coin Denominations
The second bit of duplication is in the
subject blocks. We are
CoinChanger three different times. Since we’re just
new, it doesn’t feel like serious duplication but if we ever
wanted to change the way we create a
CoinChanger, there are three
places to change.
subject blocks are also missing an opportunity to communicate
a little bit more information to the reader. What is different about
the three top-level
describe blocks? It’s really just the
denominations of the coins. That difference is somewhat obscured by
the extra noise of the
Let’s see about fixing both problems:
Now there is one subject defined for the entire test suite, and a new
instance is constructed for each individual spec using the coins
specified for that spec’s containing
describe block. The
let(:coins) definition at the top of each
communicates exactly what’s special about that context.
On the other hand, the
subject definition at the top of the spec is
a bit confusing, as it refers to
coins and we have no clue where
that’s coming from. It feels a bit like “spooky action at a
distance.” The bigger the spec grows, the more of a problem this is.
I might argue that a spec that is too big to handle this approach
might be testing a class that has too many responsibilities and that I
should focus my efforts on that problem instead.
As I mentioned earlier, there are many people who would argue that these new abstractions are not worth it. They’d argue that it makes the tests harder to understand at a glance.
If I’ve chosen the wrong abstractions, then they have a very valid point. When creating abstractions in any code, it’s important to choose abstractions that are helpful; many times, we choose the wrong ones and it makes our code more confusing to those coming after us.
I think that the abstractions I’ve chosen help to communicate the important parts of the tests while making the less important details a little bit harder to access.
I really want my tests to focus on what I’m testing, not how I’m testing it. So if I can make the “what” front-and-center and abstract away the “how”, I think it makes the tests easier to read and reason about.
Getting rid of some duplication is a nice side-benefit, but should not be the main goal. Communication should be the goal, and duplication removal can often be a way of achieving that communication.
What do you think about these abstractions? Do they help? Or have I gone too far?