We had a successful final hour of the day today. Our task for the past few weeks has been to read extra data from the XML metadata files we receive from the major music labels. One of the major labels provides their data with slightly askew logic which is different to all of the others. They prefer to omit an element to show that a given release no longer has has the value previously in that element – this requires us to know the previous state of a release to compare with the changes. This is quite a common computing problem but, when working with the type of legacy code project that we do, it can become confusing.
We’d been trying to solve this new problem for most of the day and almost had it complete, except two tests still needed to pass. We couldn’t get the logic right so that both would pass, it was either one or the other – as these were acceptance tests too, the amount of code they covered was quite large. Add to this, that we’re working on a VB.NET project that hasn’t been updated for around 4 years and you’ll see why frustration started to build.
My team mates Tony and Will had been trying to find out what was going on using the debugger, and hadn’t managed to sort it out when they asked for an additional pair of eyes. I heard about their efforts, and briefly looked at the code, but immideiately thought back to my days spent with Agile Coach Kevin Rutherford. Kevin used to tut disapprovingly every time we started the debugger, suggesting that we were just missing a test instead. I remember thinking that the fact we were missing a test was quite obvious but the steps to take to find that missing test were the hard part.
The Saff Squeeze
Today we used techniques from Kent Beck’s writings on the “Saff Squeeze”. Tony posted a quick blog about the resources we found after we’d solved the problem – it turns out we’d followed the process reasonably closely. We identified in the controller method two loops that could be extracted into pure functions (J.B Rainsberger has helped us a lot in our legacy code adventure). Once extracted we made these public and static (so they used no internal state) and we could then write a test directly of this functional part of the code. Passing in a couple of simple int arrays proved the code worked as expected so we moved onto the next loop. We followed the same process again, and a couple of simple tests asserted that these functions were doing what we wanted. This meant the problem must be with the caller, and more specifically, the value of the parameters we were passing in.
A quick look at the formulation of the variables used as parameters, and another quick change to the parameters of the tests proved that the problem was with the parameters. We’d filtered our input list incorrectly, forgetting to coalesce nulls to false as we’d missed one of the edge cases. We had tests for this class but missed this edge case.
So, with that in mind, we wrote a new test to ensure the nulls we replaced with a value of false, the parameters to the extracted functions were now correct and our two acceptance tests passed. After a bit of refactoring and exploratory testing tomorrow this MMF will be done.
Test Rather Than Debug
If I’d just strolled over to my team mates and said “stop using the debugger and write more tests” they’d have just ignored me. By breaking down the code and isolating the logic into pure functions, which we could then test, we’ve seen directly the value we got by adding these tests. The best part of it all too, is that these tests will now last for the lifetime of this code – the debugging session, once over, can add no further value.
Kevin also used to say “if you think a private method should be public, you’re probably missing a class”, and he’d be right about the two methods we extracted here. The two methods we made public so that we could test were already static, so following Fowler’s “Move Method” refactor we could safely move these to their own respective classes. These two classes now have their own behaviour, helping us meet the Single Responsibility Principle and use Dependency Injection.
Hopefully we’ll see this quick episode today as a good reason to use this technique again, and I thought I’d blog about it as evidence for others too.