My time working on the refactoring project has come to an end; at least for a while. Here’s a little look back over what we achieved.
I’ve been working on the “Refactoring Project” for around 9 months now. I’ve worked alongside the team responsible for the project several days a week for most weeks of that period. In that time we’ve tried to take a project that had become a big ball of mud, whose original developer had left, and make it more maintainable and testable and correct. Whilst doing this we had to continue to add new functionality for the users regularly.
Things have gone well. At the start I produced a report on the key issues that were generating risk on the project and we’ve been working at reducing or eliminating that risk.
The first thing we did was introduce a repeatable build. Up until that point the team used CVS for revision control but didn’t use tags for version control. Since that point we’ve done over 42 releases in 9 months. Each of those releases is repeatable and we can still build all of the distinct releases if we need to. All releases to production are done via a script which is executed with the name of the version tag and this script pulls the correct source out of CVS and builds and packages the product for production deployment.
Once we had a controlled build environment so we knew what we were releasing we started to reduce the coupling in the code base. During the original development no thought had been given to coupling. Lots of classes were coupled to lots of other classes that they shouldn’t really have cared about. Changing a single class often caused the whole project to rebuild as the header file dependencies were very coarsely grained. We’ve improved that. Classes keep more things to themselves; files only include what they really need to. Code complexity and coupling is down. Cohesion is up. Build times have dropped from 13 minutes to 9 minutes.
The next major improvement was to separate the code base out into libraries of related functionality. The original design was a single OCX which contained hundreds of source files. We started a process or moving code into libraries not for reuse but just so that the physical organisation mirrored the logical organisation. The resulting libraries were more testable because we could simply build a test harness and link it to the library of code that needed to be tested.
During all of this we aggressively removed dead code to simplify the codebase. There was lots of obviously dead code and lots of code that could never be executed because functions included unrequired flexibility that was never needed.
Once we had code in libraries we could start to test it. We were careful to introduce tests in a manner that provided the most “bang for the buck”. We didn’t aim for broad coverage, we simply aimed to introduce tests into code that we needed to fix or work on. The first test was in the data entry validation code; this allowed us to improve the performance of an existing piece of code whilst validating that we hadn’t changed the expected functionality.
Once we had a test and a test harness there was a point to doing a daily build. This was the simplest possible daily build. I adjusted our release script to be able to build and run the test harnesses and simply ran the script on the head revision of the source code every day. The daily build ran on my machine and was manually instigated, far from ideal, but better than nothing.
During the time that it took to implement all of the above processes we also spent some time managing our user’s expectations. We wanted them to know that we could release new features quickly if they really required them quickly but that it was safer for them and more efficient for us to release them to a predictable schedule. This took some time to take hold because up until this point some users had been used to calling up, having someone make a fix and for that fix to just suddenly be available in production. No tagging, no official build, no testing… Eventually they trusted us to slow the perceived rate of new feature releases down in order to increase the actual rate of new features released…
At around this time my focus shifted from the entire application to one particular area of the app; the FX trading displays and rates calculation engine. This area had a distinct set of users and they needed a lot of changes made quickly. The plan was to switch all FX trading from two other vendor systems to the refactoring project.
The FX code was a complex mess. It was complex mainly because the original developer didn’t appear to fully understand the business domain. I tend to stay pretty technically focused and only learn as much as I need to about the business to write the code I need to write; so for the work on this piece of code I went out and bought a book on FX trading.
Once I understood what we were trying to do we teased the code apart and into a library and put some tests in place. The tests for the FX code required that we mock up the data provider so that we didn’t have to try and debug code that was being driven by live market data. Once that was done we could write all manner of tests for all the complicated edge cases and drive them with our mock data provider. The user interface for the FX GUI proved harder to test. With the tests in place at the inflection point, the point where the results of the changes we were making met the rest of the code, we decided to release the current code changes before we began refactoring to clear the way for adding new functionality. We went into UAT with the new code and found that our testing wasn’t quite as good as it could have been.
Now that we had some tests to support our FX changes we refactored and added new functionality and from that point on things went well. The tests gave us confidence and we added new functionality quickly whilst fixing bugs and generally improving the quality of the code. The users were very pleased with the speed that we could react to challenging new requests.
And that’s about it. There’s a lot more to be done on the project but now is the time to leave the team to get on with it alone. The users are happy. The client is happy and we may rejoin the team to help them out some more some time next year.