I was recently in a conversation about some significant refactorings that had happened in our codebase. There were a couple of opposing viewpoints about the value of those refactorings:
So, where do I stand on the issue? I agree with both points. I think it’s important to refactor the code for the reasons above, but also to generally simplify it over time. The key to me, is how you approach refactoring a codebase.
There are a few things to consider, before, during, and after you complete a refactoring.
In the excellent book “Working Effectively with Legacy Code” author Michael Feathers defines Legacy Code as code without tests. He goes on to describe that the secret to working on legacy codebases is to bring them under test. This is the same thing you need to think about when you’re planning a large refactoring. Before embarking on the refactoring, understand what tests already exist? Do they cover all of the major scenarios? If not, in order to refactor responsibly, you need to add tests. A common concern with writing tests before refactoring is that the refactoring is just going to break the tests anyway. My opinion is that if that is true, the tests weren’t very high quality to start with. Ideally, tests should characterize the customer observable behavior of the system, without being tightly coupled to the implementation details of the code. Building mocks that ensure an exact sequence of calls is typically less useful than a test that operates a larger chunk of your code with only its external dependencies abstracted.
Consider whether you can write tests that act as an oracle or provide a baseline of expected customer results. This can allow you to significantly change the implementation while still ensuring that customer behavior is preserved.
Code coverage can be a tool to help identify scenarios that you haven’t considered. I’m not a big fan of trying to hit specific code coverage percentage targets, because I think that incentivizes the wrong things (a topic for another day). However, if determining what parts of your code are covered by your existing tests is possible with a reasonable amount of work, reviewing those results to find areas of code that aren’t covered can be a good way to identify where you should focus your efforts on increasing test coverage.
Once you have the tests in place, you can much more confidently proceed with your refactoring. Of course, it’s unlikely that you’ll have a test for every possible case, so diligence is still warranted.
During refactoring, consider separating completely mechanical, automated refactoring operations into discrete units of work (a separate PR if you squash, or a separate commit if you don’t). Many of today’s developer tools have sophisticated and robust refactoring operations for things like Rename and Extract Method – I even worked on the ones for C# back in the day. Most of the time, if your tooling does an operation for you, you can trust that it does a pretty good job, and so separating it out makes it easier to skip over when looking for the culprit if a regression is found once your code does go to production.
So, you’ve implemented tests, you’ve refactored the code, created a Pull Request, gotten it reviewed and merged, time to celebrate, right? Well, slow down there a bit. Now that you’ve refactored this code, you should feel increased ownership. It’s important to remember that that ownership lasts through the whole life cycle. Once you’ve merged your refactoring, make sure you understand how and when it gets deployed. Follow it through whatever systems your team has until it's live and being used by customers. Watch for indications that something went wrong, and proactively attempt to diagnose and repair them. Ideally, your on-call team isn’t surprised by an issue because you’ve already noticed the signal and addressed it. If they are, make sure you are available to help troubleshoot. Be accountable for the quality of your code right through the deployment process until it’s vetted by customers.
As I said above, refactoring is an important way of improving the quality of a codebase, helping people new to the code base learn it, and engendering a sense of ownership. However, it’s not without risks, and shouldn’t be taken lightly. Consider what you can do to make refactoring as safe as possible. Your team, and your customers will appreciate it!