One of the things that I noticed in the last couple of years is that a significant number of .NET developers (including myself) are doing nothing more than procedural programming while using objects. The more I’ve worked on large projects, the more I’ve noticed that this approach imposes a large set of restrictions on what you can do with your code. The main project that I’m working on right now is fairly large and has some interesting complexities in both the business rules that we need to program for as well as the architectural paradigm that we work within.
After spending some time looking at areas of that application that the development team shuddered at working in, I realized that the procedural nature of the code was restricting our ability to automate tests. A direct relation to this problem was the lack of adherence to the Single Responsibility Principle. This is not to say that procedural code will always turn its back on the SRP, but in the situations that I was looking at the two went hand in hand.
In my reviews I was finding methods that contained code that covered dozens and possibly hundreds of execution path permutations. It was obvious that the original and maintenance developers had noticed the extent of these paths when you looked at the test that had been written. In the cases where there were to many obvious permutations to even consider counting them, tests were simply not written. In other cases, where the complexity of the execution paths was more understandable and quantifiable, automated tests never covered all situations. Instead only the scenarios that appeared “most likely” were handled, which amounts to a system who’s complexities are the least tested areas. As a result the untested and partially tested code was very brittle and very frightening to a developer who had to modify it.
Don’t get me wrong, there were some definite benefits to the code. For instance, the one function that had a few dozen simple ‘if’ statements (embedded eight to ten deep) made it easy to do two things: tie the code to the logic explanation in the story, and it was easy for a new, less experienced developer to grok. Another thing that I found was a mentality that more classes was a bad thing, so developers knew that the functionality for invoice calculations existed only in four or five different classes and files. Beyond the fact that the readability was fairly intuitive, not much else about the code made me smile.
We’ve spend some time over the last few weeks refactoring core portions of the application and making them more testable. The one experience that I’ll talk about the most here is a situation where the maintenance developer had a bunch of free time and an area he was very knowledgeable of needed a lot of help. He started working on refactoring the areas of one business component that had a high levels of complexity, cohesion and defect reports combined with low levels of testing, SRP adherence and both client and developer confidence.
When we went through the existing code I made the decision to direct the refactoring effort towards SRP and Dependency Injection. This developer, although bright and with a number of years of experience under his belt, had never heard of Dependency Injection or SRP let alone written code that used them. We found a piece of code where the execution paths were numerous and deeply nested, but not very complicated when you looked at each individual piece, and decided that it was as good a place as any to start. By the time the developer had finished the refactoring we had gone from 1 method in on class (breaking SRP due to the method really having nothing to do with the purpose of the class) to about 25 classes, each adhering to SRP and implementing Dependency Injection. That one original method had less than 10 tests written for it, none of which were what I’d consider confidence building and in combination they certainly didn’t exercise all of the code in the method. What we have now is a suite of 300 to 350 tests which test data state through NUnit assertion and expected delegation through Rhino Mocks. Because I haven’t run nCover against the code yet I haven’t been able to prove this, but I’m very confident that our code coverage has increased substantially. More importantly though, I believe that by focusing on SRP while designing the code we have been able to write much more effective tests.
After the refactoring exercise was over I asked the developer if he thought it was worth the effort. He immediately responded that it was and that the effort wasn’t as large as he’d originally thought when starting out. The best part of the response was the enthusiasm that he spoke with when talking about this piece of code. Prior to starting this task, developers and BAs would physically cringe (that is not me embellishing either) when there was any mention of the business process that we refactored. This developer was no different (if anything he cringed more than most). While he was sounding off about how worthwhile the previous weeks work had been I noticed the one thing that changed more than anything was his confidence in the code. Even though the code read with more difficulty due to the repeated delegation from one class to another, this developer definitely exuded confidence that future defects would be easier to track down and that we would be spending less time working within it.
I love to work with devs and show them new techniques or teach them new technologies. I find it to be one of the most rewarding parts of my job. Of all the time I’ve spent in this career, the last couple of weeks spent working on this solution has been the most rewarding and enjoyable I’ve spent in legacy code.