Taking controlPosted 2012-01-19
I'm sure you're familiar with this situation, and if not, you're either lucky or new: Starting a new assignment and taking over someone’s code, code that is in some sort of state of decay or disarray. Bonus points if the original author isn't even around anymore to answer any questions you have. Extra bonus points if there's no source control.
I've done this scenario quite a few times now, and the experience for me has gone from horror and bewilderment the first time I did it, to actually now enjoying it. As long as you have a proper and well thought through approach, I'm positive you can turn almost all code around.
What about rewrites then? It's almost a mantra that if code is old enough it should have been rewritten. I'm sure there are times when this is the right choice, but it carries a very high risk. Going through code you sometimes find really odd pieces, which seems to do elaborate things for obscure reasons. Those, ugly as they may be, are sometimes gems of knowledge on how to handle specific corner cases. If you rewrite things, you will lose those gems. Another huge risk is the time when rewriting and you still have to manage the old system in addition to rewriting it. With features actually going in to the old stuff while writing new, and the split focus; you're threading on perilous ground. Ask the Netscape guys, they should know.
So, refactoring is, in my opinion, the way to go in most cases. I'm going for the old-school definition of refactoring here - changing the implementation without altering the functionality. Don't try to improve on what the thing actually does when you refactor. Just make it pretty on the inside. My approach to this is almost formulaic by now:
First off the bat, write tests for the existing code base - try not to change the code base too much. If any refactoring is to be done, it should be in order to increase testability. Aim for a coverage percentage you feel comfortable with, but don't aim too high. As long as the coverage percentage doesn't become a goal in itself - there's quite a few horror stories about what has been done to increase code coverage. I remember hearing once about a guy who wrote a function that just iterated through every available function, calling it with null or 0 and ignoring the result and catching all exceptions. That sort of test is even worse than no tests whatsoever, since this leads to false security. Refactor the unit tests as well. I sometimes wonder why test code doesn't get the same refactoring love that "normal" code does. It deserves it, test code is the only code that will find bugs instead of causing them.
Then refactor. The important thing is to not try to force the code into something it was never intended to be, say moving a webforms app to a MVC app while refactoring. That's in the realm of rewriting, not refactoring. Instead, make it the best webform app you can. The point is to step-by-step make the codebase smaller, and thus more malleable. I usually find it easiest to work from the bottom up, starting with the data layer and ending up in the UI. As long as the refactoring never takes the form of scrapping an entire module of code, you should be able to continually have a working product throughout the process.
Use a good tool! I like ReSharper. Consider style warnings for your tool of choice as errors and accept no deviations. Spending time adding features like a keyhole surgeon to an uncontrolled codebase is producing waste and, more importantly, consolidating broken code.
That is just my two cents...
Shameless plug: Are you a ninja? We are hiring in all our offices.