Lately I’ve been tracking some of the steps I go through in a given day or week. I was fixing a bug the other day when I decided that I should write down all the mental notes I refer to when addressing a problem like a bug in code. These are questions I didn’t always ask in the past and I feel they are valuable, so why not share them?
Here’s a few questions I’ll ask myself when I think I have fixed a defect.
Does This Solve the Problem?
The first and foremost concern is fixing the problem. Do the changes I’m proposing actually fix the problem? Or do they defer it off elsewhere? I try to remember that “the fix” should actually fix the problem. This may seem obvious, but sometimes a bug fix only deflects the problem elsewhere.
Is This Flexible, Maintainable, Clear and Simple?
Some of these may seem to overlap, so I’ll describe what I mean by each of them:
- Flexible: Does this solution allow somebody else to easily change or add to the code I have written? I could probably fix an issue by adding a bunch of conditionals in a place they don’t belong, but this is just duct-taping the broken code.
- Maintainable: Similar to the previous question, does my solution address the root cause? I could run a database query to correct some bad data from time to time, but this doesn’t lend itself to fixing the root of the problem. It creates a maintenance nightmare and just ends up being a waste of time.
- Clear: Are the changes I have made obvious and discoverable? I work primarily in the ASP.NET world and there are many ways to write code that isn’t obvious (what comes to mind right now is writing an HttpModule that does some work to modify a request/response). Slipping in some code that fixes a problem at an odd place is not a clear way to fix a problem.
- Simple: Do your best to stick to the simplest thing that works. I could implement a big system to address a problem that involves an editor and management tools. However, if I can fix the problem some of these won’t even be necessary. Don’t over-engineer the problem to make it a bigger one than it actually is.
Does This Leave the Code Better Than I Found It?
Somebody once said ”Always leave the code in a better state than when you found it.” That somebody was Robert (Uncle Bob) Martin. It’s great advice and when followed, you’re creating a better product every day. I’m a firm believer in this motto because it works. Don’t take shortcuts, you’re only going to be spending long nights later on paying back the technical debt you’ve accrued.
Is It Covered with Tests to be Future-Proof?
How do you prevent defects and bugs from popping up again? There are two ways I can think of. You write unit tests and keep them running and passing with every code change, or you hire a team of people to test every possible feature of the code every time a new feature or bug fix is added. Your most consistent option is also the cheapest, the former, unit tests and a CI environment. Write a test to confirm the defect then fix it. If you continue to keep your code in a consistent state of harmony, you’re going to prevent defects from occurring and re-occurring. I could also point out that fixing a bug right away is 1000 times cheaper than fixing it after it goes into production, but you probably already knew that. ;)
The previous four questions are running through my head the whole time I’m addressing the issue. Once I feel I have them all satisfied, I go through a few more steps before having my code peer reviewed:
Is There Any Code that was Commented Out or Stale?
If you’re on board with never commenting out code I’m partially with you. I’ll comment out code when running tests, but the real problem with commenting out code is getting that code checked into source control. I’ll browse through my changes and make sure that I don’t have any code that is commented out that I forgot to delete. While this has the benefit of maintaining a clean code repository, it’s also quite embarrassing when somebody has to ask you to delete it because you forgot.
Additionally, productivity and/or refactoring tools may give you warnings of code that is unnecessary, unreachable, or just plain pointless. Delete those lines too, they’re just making your code messy and nobody likes cleaning up a mess that isn’t theirs.
Do Any Named Variables or Classes Exist that Aren’t Clear?
This one isn’t always easy for me. It’s hard to take yourself out of context for a moment to ask yourself if the names you have chosen are clear without knowing the bowels of the code. A class with a name of LabelRequest might make sense to me since I’ve been working on a customer product return service, but there could be a better name for it. Something like ReturnShippingLabelRequest is a bit more clear for somebody else entering that area of code for the first time. Either of those beats “LR” though.
Are All Changes Relevant to the Fix?
Lastly, it’s important to ask yourself if all code changes that are being reviewed are applicable to the fix. Make sure there is nothing like “I also cleaned up 15 other files” going on in your request for review. These are red-flags that a code reviewer should probably point out to you, but if you can catch them before somebody else, you’re just saving some extra time for everybody. Additionally, if I’m reviewing thousands of lines of pointless code changes mixed in with some critical code changes, I’m probably going to get lazy with the review and miss something. Keep it relevant and you’ll keep the code reviewer astute.
By no means is this list all-inclusive and perfect; I’m just sharing what is currently working for me and my team. Feel free to leave me any comments or suggestions on thoughts/questions you consider when fixing a defect.