Getting stuck on leap years with TDD
The other day I was introducing a colleague to TDD using the leap year kata. After getting the first few tests green, we got stuck. Turns out doing TDD is not the same as teaching TDD..
What I got stuck on was that when it started to be unreasonable to hardcode the years, I couldn't figure out how to proceed. There was no obvious duplication to extract, and there was no obvious way to justify introducing the modulo operator.
After some thinking I realized that I was trying to force the solution towards the end goal, instead of letting the solution emerge from the tests. So let's try again, shall we?
def leap_year(year):
return True
def test_the_year_4_was_a_leap_year():
assert leap_year(4) is True
The simplest test I can think of is that year 4 was a leap year. So I write that test, and make it pass with the simplest possible implementation. Next up, let's the year 1 as an example of a non-leap year.
def leap_year(year):
if year == 1:
return False
return True
def test_the_year_1_was_not_a_leap_year():
assert leap_year(1) is False
All years are leap years now, except for year 1. I think I'll need another example for non-leap years. It's tempting to go for the year 100, but that's a different rule. I think I'll go for year 2, which is also different from year 1 by being an even number. That leads me to write:
def leap_year(year):
if year == 1 or year == 2:
return False
return True
I could make my code slightly more general, by checking for numbers under 4, since my code is starting to violate the heuristic of "tests become more specific, production code becomes more general". I feel like I've fallen for this trap before though, so maybe one more example? Writing a test for year 3 gets me there. Here's the current state of my code:
def leap_year(year):
if year < 4:
return False
return True
def test_the_year_4_was_a_leap_year():
assert leap_year(4) is True
def test_the_year_1_was_not_a_leap_year():
assert leap_year(1) is False
def test_the_year_2_was_not_a_leap_year():
assert leap_year(2) is False
def test_the_year_3_was_not_a_leap_year():
assert leap_year(3) is False
Let's start on a larger number now. Year 5, maybe? I could go a couple of ways with that. I could replace the if year < 4 with if year != 4, or I could go with if year % 4 == 0. The latter is more general, and the first actually makes my production code more specific, so I feel like the modulo operator is justified now. Actually, Python being Python I don't even need to check that the result if year % 4 is 0, since all numbers over 0 are truthy. So here's the new code:
def leap_year(year):
if year % 4:
return False
return True
Now let's see if I can get to the next rule, which is that years divisible by 100 are not leap years. I'll write a test for year 100 now, which leads me to put an or in the if statement:
def leap_year(year):
if year % 4 or year == 100:
return False
return True
Let's write another test to try and get rid of the hardcoded value:
def leap_year(year):
if year % 4 or year >= 100:
return False
return True
def test_year_200_was_not_a_leap_year():
assert leap_year(200) is False
As you can see, that doesn't really get me there yet. I'm struggling a bit now, because I can't really think of another failing test case for the first two rules. A test for 101 being a non-leap year passes, and so does a test for 300.
It's interesting what writing does for your thinking, because as I just wrote that, I realised I should pick a leap year over 100 for the next test. Asserting that 104 was a leap year fails now, and I can solve this by reaching for the modulo operator now.
def leap_year(year):
if year % 4 or year % 100 == 0:
return False
return True
Because I find myself getting confused about how I've actually implemented the rules, I think it's time to do a bit of refactoring. I probably should've done so, earlier.
def leap_year(year):
def is_divisible_by_100(year):
return year % 100 == 0
def is_not_divisible_by_4(year):
return year % 4
if is_not_divisible_by_4(year) or is_divisible_by_100(year):
return False
return True
The problem lies is the fact that I'm using a negative condition for the first rule, and a positive condition for the second. This makes it hard to reason about the combined condition. Maybe I'll make that more explicit first, before doing something about it.
def leap_year(year):
def is_divisible_by_100(year):
return year % 100 == 0
def is_divisible_by_4(year):
return not year % 4
if not is_divisible_by_4(year) or is_divisible_by_100(year):
return False
return True
I'm not sure if this is truly better, but I think I'll get there after implementing the last rule: that years divisible by 400 are leap years. Let's start with a test for year 400 now. That gets me to:
def leap_year(year):
... # some content omitted for brevity
if year == 400:
return True
if not is_divisible_by_4(year) or is_divisible_by_100(year):
return False
return True
That works, but my code is becoming more specific once again. I'll need a couple more tests to get me to a more general solution. Let's assert that the year 800 is a leap year, too. That leads me to:
def leap_year(year):
... # some content omitted for brevity
if year >= 400:
return True
if not is_divisible_by_4(year) or is_divisible_by_100(year):
return False
return True
That's a bit more general, but obviously wrong. Let's test another edge case. I could go for 401, or 500. 401 is not a leap year because it's not divisible by 4, and 500 is not a leap year because it's divisible by 100 but not by 400, so that should help me. Both are good edge cases, but which takes me the smallest step closer? After experimenting a bit, I don't think it matters. I'll go with 401 because it's a smaller difference to 400. That leads me to change if year >= 400 to if year % 400 == 0. And that should do it. My function now looks like this:
def leap_year(year):
def is_divisible_by_100(year):
return year % 100 == 0
def is_divisible_by_4(year):
return not year % 4
if year % 400 == 0:
return True
if not is_divisible_by_4(year) or is_divisible_by_100(year):
return False
return True
It's correct, but some refactoring is in order. One fewer return statement would be nice, since there are only two possible outcomes. Here's one attempt:
def leap_year(year):
def is_divisible_by_400(year):
return year % 400 == 0
def is_not_divisible_by_100(year):
return not year % 100 == 0
def is_divisible_by_4(year):
return not year % 4
if (
is_divisible_by_400(year)
or is_divisible_by_4(year)
and is_not_divisible_by_100(year)
):
return True
else:
return False
That's fine, but I'm going to have some fun with it now and see if I can make it feel like a DSL. What about:
def leap_year(year):
class the:
def __init__(self, year):
self._year = year
def is_divisible_by_400(self):
return self._year % 400 == 0
def is_not_divisible_by_100(self):
return not self._year % 100 == 0
def is_divisible_by_4(self):
return self._year % 4 == 0
the_year = the(year)
if (
the_year.is_divisible_by_400()
or the_year.is_divisible_by_4()
and the_year.is_not_divisible_by_100()
):
return True
else:
return False
Well, I could go on. Perhaps I would even change the API from a function to a Year class and go with something like Year(400).is_leaping(). But I think I'm done here, and there's not much point in experimenting further I think.
Conclusion
TDD is hard. Getting stuck is part of the process, but it's important to reflect on why you got stuck, and how to get past it. Letting the solution emerge from the tests, instead of trying to force it towards the end goal, is key. I also think I was feeling uncomfortable showing my colleague a toy problem thinking it would not highlight the advantages of TDD. TDD, and many eXtreme Programming practices (or agile in general) are counter intuitive to many developers, which is why there's always resistance to these ideas. Turns out we hired very well though, and my new team mate is very open to trying new things.