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.
The other week I inadvertently coined a new phrase. It may actually be two phrases and comes from our recent concentration on abstraction in our controllers and main calculation engines. I just thought they were one of the silly things I say to help turn programming concepts into easier to use language – much like we use design patterns to communicate intent when pairing or discussing architecture. I was encouraged to blog about it by my colleague Shaun, who thinks this might go global.
Phrase 1: “A Sea Of Blue”
This is what we see when we open up a controller or engine that has a number of service objects injected in the constructor. The name came from the fact that we work in C#, in visual studio and our theme colours interfaces in light blue. It is generally regarded as a good thing as the class in question will be more testable as the services you don’t need can be easily mocked. We can return specific messages from some services, verify that others are called in a specific way and provide a mock with no setup/verification to skip over unwanted behaviour.
We’ve found that sometimes the sea of blue means that you have a long parameter list. We usually combat this by defining DDD style aggregates for service objects with related behaviour and injecting an aggregate service instead – maybe these are more a lake of blue – or maybe I’m getting carried away.
Phrase 2: “Making a Bluey”
This one is probably less likely to catch on, especially as most people think of something completely different when you say “bluey”. In other words, extracting an interface, or Ctrl+Shift+R and “Extract Interface” if you want to be fancy. We’ve had discussion recently whether we sometimes do this too soon – do you need an abstraction unless you need two concrete implementations? I often argue that the second implementation you have is the mock implementation you use in tests. My colleague (Shaun again) blogged about the way we should mock roles rather than types and one of things we’ve discussed before is that you should understand why you use a tool like Moq by actually implementing a mock/stub of the class rather than using Moq. This makes it more obvious that you now have two versions of the abstraction in existence and therefore the previous argument no longer stands.
I like the concept of emergent design and the use of test driven design to define the api of a class. I think this extraction of an interface, so that it can be mocked effectively and used without worrying about the concrete implementation, is an effect of the tests driving the code. The resultant api is cleaner, the classes can be well tested and your code can follow the SOLID principles.
I’ll get our very latest version of the theme available soon on the company blog but for now you can try the old version, which works well in Visual Studio 2008 but because of the open type font restrictions in WPF and therefore Visual Studio 2010 you can no longer use inconsolata and some of the colours need updating:
Our new theme includes updates for 2010 and resharper 5 too.