Thursday, 1 May 2008

The Great Refactoring Debacle

The Vision

I have been giving some technical seminars to the rest of the development team at my workplace every few months, trying to keep them up to date with good coding practices and the latest technologies. While a few of them are already quite interested in these topics, on the whole most are simply content to plough on with the knowledge they have and only learn new stuff when an obstacle confronts them. But on the whole, my presentations have been well received.

It was a few months ago that I brought up the subject of refactoring. Twenty developers all coding away merrily on the same project for two years things had resulted in some less than ideal code. It was not as if the product was low quality. Far from it. Our bug count was low and the customers who were using the product hadn't escalated any major issues to the development team.

But there was a growing awareness that the codebase was getting a little out of hand, and management agreed. We were going to spend a few months simply working on the quality of the code, ready for a big "general release". No new features, just bug fixing and "refactoring".

The Strategy

This was a "once in a blue moon" opportunity to right some of the wrongs from earlier phases of the project. I presented my opinions on where I thought our efforts would be best spent during our two months of refactoring. We would focus on code smells, breaking huge classes and methods down into smaller pieces that did one thing and did it well.

At the same time, the more daring developers would try to eradicate some of the very tight coupling that had crept in, and to separate the GUI from business logic. Without these two advances, any hopes of running automated tests would be merely a pipe dream.

Finally, there were some classes that were being used in multiple places resulting in it being far too easy to inadvertently break someone else's code. I proposed to separate this code off to form some new extensible components that could be customised and used in different ways by different parts of the application.

The Refactoring

So we got to work. The build-breaking changes went first. Thousands of lines of code were changed. Developers were delighted as they successfully turned previously intimidating classes into groups of much more manageable code. Some of the worst-designed features of the application gradually morphed into a more streamlined and extensible framework.

There wasn't time for everything. In fact, some of my top recommendations had to be left to one side. And other ideas just proved too difficult. For example, I tried to extract some groups of Windows Forms controls from a Panel containing thousands of controls, but the process was just too fiddly (if only one of the refactoring tools could do this).

The bug count dropped dramatically. In fact, it dropped so fast that most of the development were seconded to test to help them catch up. All was looking good.

The Aftermath

Then came the system test. The first "full" system test in over a year. And there were lots of new bugs found. On the whole we managed to stay on top of them, but as the release date grew closer and closer, some "showstopper" bugs came out of the woodwork. The release date slipped three or four times and ended up coming a few weeks late. The overrun wasn't huge by comparison with many other projects, but an explanation was needed.

I wasn't in the meetings where blame was apportioned. In fact I was on holiday that week, but it was clear when I returned that a scapegoat had been identified. Refactoring had caused us grief, and perhaps we shouldn't have done it. On the surface of things, it was the refactoring that caused some of the very defects that delayed the release.

Now I could get on my high horse and demand a chance to stand up for "refactoring". I could point out that...

  • It's not really refactoring if you don't run unit tests (because you don't have any unit tests). It's simply re-architecting which is very risky.
  • Refactoring is always best done on code that you are actively working on. You understand what it is doing and why, and you have already allocated some time to testing it. Diving into a class and refactoring it simply because it is "big" is also risky (especially if you don't have any unit tests).
  • Refactoring is primarily about making small, incremental changes. Over a long period of time the structure and design of the code should improve. Trying to do it all in one hit is risky.
  • Refactoring does introduce bugs from time to time, because all modification to code risks introducing bugs. That is equally true whenever you add features or fix bugs. Indeed, it could be argued that the new features that "sneaked in" to the refactoring phase were the cause of a lot of our problems.
  • The main rewards of refactoring are felt in the future. For example, new features can be added much more quickly. Bugs can be found and fixed much more easily. In fact, my team is already reaping the fruits of the refactoring in the new feature we are adding, but this kind of benefit is not highly "visible" to management.

... well I could point these things out, but I suspect it would just come across as whining because my idea of "refactoring" apparently didn't "work". And if I say too much I risk someone deciding that refactoring should be "banned" because it is clearly too dangerous.

So I'll keep my mouth shut, and continue to improve the codebase bit by bit, sticking to the code I am already working on, and adding unit tests as I go. Management won't credit any resulting quality enhancements or development speed increases to the discredited "refactoring" idea, but at the end of the day, much of my job satisfaction comes from the knowledge that I have contributed to a quality project.

I suppose the bottom line is that the benefits of refactoring are not easily measurable. Bug counts are measurable. Completing features is measurable. But what is not measurable is how much refactoring has contributed to the speed at which bugs are fixed and new features are added. And as the saying goes, "if you can't measure it, it doesn't exist".

Technorati Tags: ,
Post a Comment