Code refactoring: helping your team help you
Code refactoring is a fact of life. It’s eternally necessary in order to maintain modularity, readability, and testability. But, it can be risky business.
Maybe you’re new to software engineering or new to a particular development team, and you’re prone to noticing where substantial improvements can be made. The difficulty in this situation is that you may not have a well-established reputation amongst your new team and your confidence in the new code might be low. Or, maybe you’re a seasoned veteran and people have come to trust your code—perhaps a little too much.
Having stayed humble over the years, you realize that refactoring can be fraught with problems, and you might want a bit of extra reassurance that your work isn’t introducing any regressions. There are so many reasons that introducing a major refactor can be a bit of a nightmare. To make it less so, you can lean on your team to ensure things go smoothly. And, there are ways to make life relatively easy on them while sharing this responsibility.
My first refactoring project at Workiva
When I first joined Workiva, I took ownership of some small improvements to a library used heavily by Wdesk. These were introductory tasks to get me acquainted with the code base—tasks that would have a real, if small, impact for consumers. But, as small tasks often reveal, there was some refactoring work that, if completed, would make the code more readable, maintainable, testable, and, most importantly for a newcomer, understandable.
In a former life, this wouldn’t have fazed me, but I already knew that at Workiva, we place a premium on quality. Consequently, my refactor was going to need to be seamless.
Luckily, my team had already created a safety net of unit tests around the code I wanted to change, but any feeling of safety it provided was short-lived: the structure of the code changed substantially enough that the test suite no longer made sense. In particular, I’d needed to split one very large method into smaller, more cohesive, and testable parts, allowing the original method to delegate responsibility to the smaller methods.
The new methods had shiny new test classes to prove they did what was intended, but the test class for the original “big bad method” no longer made sense. Its arms were reaching out into external methods, it was mocking out code outside the module under test, and each test occupied a great many lines of code. Although this class still technically proved that my new code was working, it was no longer clean, concise, and contained. I needed to rewrite it.
My safety net had fallen into the abyss. And, as a woman who likes to maintain control over software engineering and life alike, I started strategizing ways to get my work done without jeopardizing my sleep (I’m also a woman who needs her sleep).
To Google! Surely, there would be plenty of hits to help me refactor unit tests with confidence. Negative. I was disappointed to find scarcely any results. I was also disappointed to find that Twitter was full of developers brazenly declaring that they were refactoring unit tests with #NoSafetyNet. These individuals didn’t value sleep as much as I do.
I decided this was no time to be brash; it was a time for some extra attention and care.
Even if I was confident in my changes, I needed my teammates to feel the same. I was keenly aware that I was the newcomer trampling around not only in their code, but in their test suite. So, as I stomped around—albeit very carefully—I took note of the things that would help my team to understand why I was making changes, what I was doing, and ultimately, how to have trust in my work. Any effort I made to simplify their jobs would increase my confidence in my own work, and most importantly, help them to find any shortcomings.
Four keys for code-refactoring success
Resist the urge to refactor everything. In the example I’ve been discussing, I limited myself to refactoring only the “big bad method” and its corresponding tests. Did this mean all other areas of our library were perfect? No. But, I picked my battle and saw it through. This still amounted to a PR with 1,310 added and 1,069 removed lines of code. Consequently, my reviewers appreciated that I hadn’t wandered outside the boundary of my initial goal.
Here are my four keys to code refactoring success:
- Write a courteously-detailed PR description.
By this point, you’ve likely become the expert on any code you’ve touched. It’s easy to assume that others have the same level of understanding as you do. When writing your PR description, explain what you need to in order to give everyone a good understanding of your work—don’t leave out details assuming people already know them. Also, be sure to include a description of the motivation for your refactor.
- Annotate your PR inline.
Anywhere you’ve done something others might consider odd, complex, or dangerous, consider adding an inline annotation with a brief explanation. For example, if you’ve deleted some test cases, include your rationale for doing so. Or, if you’ve simplified a class by adding a new method, point this out so the diff makes immediate sense. If you find that you’re having to annotate your PR heavily, your refactor might not be ready for PR.
- Provide clear, understandable proof that you've maintained the meaning of any refactored tests.
Don’t wait to be asked for this, and don’t assume someone else will perform this work for you. Something as simple as a two-column table tracking old tests versus new tests and noting the reason any old tests have been removed can go a long way in making the case. A link to this table can be included in your PR, and at least one of your reviewers should verify it.
- Be patient.
People may have a lot of questions or suggestions about your changes. They’re right to, particularly in the case of refactored tests. It’s far better to allot some extra time to these types of PRs than to inadvertently push through regressions.
What I’ve learned
I love refactoring code, even if that code is a suite of unit tests. And, I’ve learned that my team is one of my greatest assets in ensuring I’m making our code better. I’ve also learned that mindfulness, courtesy, and patience can make my teammates’ lives easier, thus helping them to find anything I’ve overlooked.
In the case of the refactor that sparked this learning, the team gained understanding of my work, and I gained peace of mind. The code has long since been released into the wild and is behaving precisely as it always did—and I’m sleeping like a baby.
About the Author
Colleen Hardie is a Software Engineer at Workiva. She is keenly interested in the artisanal side of computer science, particularly in refactoring and testing.