It's that time again - time to go through the bug list and do some cleanup. As I work through these bugs one by one, I've noticed that at least half of them end up being slightly more than just bug fixes. As I crack the hood and peer in, sometimes I see that there's code inside that's not quite up to the standards of the rest of the application.
When I see this, I generally try to fix the whole bug, and that means cleaning up (refactoring) the messy bits. In some cases, you're working on an application that someone else wrote, and in these cases it's pretty easy to blame the mess on the moron who coded it in the first place. But what if the moron was you? How do you end up with code that needs this sort of attention if you wrote the stuff originally? There are tons of great articles and books on refactoring, and they cover this area in much more detail, but here are some of the things you're going to run into when fixing bugs:
- Code built upon code. This is probably the most common reason for refactoring. None of the code you wrote was wrong when you wrote it, but Agile development stresses development one feature at a time, and this sometimes means that you're going to add new code on top of old code and create situations where common functions need to be pulled out, base classes need to be created, and so on. These things should really be done during development when the problem can be seen for the first time, but sometimes they're missed. When you see these needs during a bug fix, don't ignore them - they won't go away on their own.
- New patterns that haven't been implemented everywhere. This is a similar concept on a broader scale. Often, we find better ways of doing something during the course of a project - a way we want to organize code, a set of CSS styles that we want to implement all over, or something like that. Again, we really should take the time to go back and retrofit all of our existing code when we agree on a new way to do something, but sometimes that doesn't happen. And again, if you're working in a section of your application that was "grandfathered", take the time now to bring it up to spec.
Why bother cleaning up this other stuff if it was working? For one thing, it's just the right thing to do. If you don't take care of your code base in this way, your application is going to be legacy code before you go to production. More fundamentally, though, is that the code probably wasn't working as well as you think if there are bugs logged against it. Maybe the "old pattern" followed in this code is susceptible to problems that were fixed in newer code. Maybe the code is more complex than it needs to be, allowing bugs to hide in the nooks and crannies. Part of a good bug fix is to look for ways to prevent a bug like that from ever happening again, and very often, this means refactoring so that the "fixed" code can be painted onto the rest of your application.
There are times when you don't really want to rip up drywall in order to fix a surface blemish, of course. If you're late in a release or making a production fix, you absolutely do not want to introduce any unnecessary instability. This means you'll keep your fix as small as possible to limit the possibility of introducing new bugs with your fix. Do yourself a favor, though - leave yourself a todo note in the code or a bug in your tracking system so you're reminded to go back at a more appropriate time and clean up.
The next time you fix a bug, take a little time to look around and look for the right fix - not just the quick fix. You'll end up with a better code base in the long run.