I was recently reading some of our project’s code before adding in a new feature. As is typically the case, I felt the burning desire to make the existing code easier to understand. However, refactoring code without a specific purpose can be a controversial topic.
Here’s my take: generally speaking, it’s a Good Thing to refactor code, provided that the changes are not significant (of course, the more well-covered the code is by unit tests, the more leeway you have in what would be considered “significant”). Everybody on the team benefits from code that is easier to read.
On the opposite side of the fence, the most commonly-used argument boils down to “if it ain’t broke, don’t fix it”. But there’s another angle that I hadn’t previously considered: team politics.
For better or worse, most organizations end up implicitly viewing the code as being owned by individual programmers, or at least different teams. Because of this, there are a couple of things that can happen when you modify code that you (or your team) didn’t write.
- Hurting programmer pride: As programmers, any code that we write is a personal mission to solve the problem at hand. When another developer comes along and modifies that code, it’s like telling you your code sucks. Hey, maybe it does. But not many people like being told that, especially when the change itself doesn’t solve a specific and objective goal. If you work with a lot of proud people, constantly making changes to their code won’t boost your stock within the team.
- The defect blame game: If you’ve been in the industry long enough, you know exactly what happens a new bug comes in: “Oh, who wrote that code? Give it to them.” This is a side effect of strong code ownership, though not without good reason: typically, the programmer who wrote the code causing the bug is the one who has the most knowledge about the subsystem, and that person will be able to fix the problem the fastest and most effectively. But if you were the last person to touch the code — even if it was a simple as fixing some indenting — there’s a very good chance that the bug will end up on your plate. Not only is this something you probably don’t want to happen, but as someone who probably doesn’t know the code so well, it will take you longer to fix the issue. This is bad for the team as a whole.
These are good reasons to avoid excessive refactoring. But being someone who believes that the advantages of refactoring generally outweigh the disadvantages — especially in the long-term, and especially when managed appropriately — I am still interested in finding the most appropriate way to ensure it is done.
I’ll put forth a couple ways to assuage the political resistance to refactoring other people’s code:
- First off, any superficial refactoring of existing code must follow a defined coding standard. It doesn’t particularly matter what that standard is. Tabs vs. spaces? Who cares! If you have a standard, then any changes you make have a valid reasoning. If it violates the standard, then it is broke, so you do fix it. Also, developers can’t technically be offended; instead of just changing the code, you’re fixing it by bringing it up to standard. It’s more of a favour than a slight (Disclaimer: this doesn’t apply to all developers).
- Second, if you do decide to refactor, make sure you do it on its own branch, and clearly label the change as only including refactoring (as opposed to including possible breaking changes). This way, when others come along to see who introduced a defect, they know that you’re not [necessarily] the one with the best knowledge of the subsystem. Additionally, keeping the change completely separate makes it easier to revert.
As a final note, I think the most important factor in resolving these political issues is communication. Don’t sneak your changes in behind people’s backs. Don’t be a lone wolf. As part of a programming team, it’s important that everybody works towards the same goals. Don’t get me wrong: cleaning up existing code should be one of those goals, but it can only be done successfully if people agree that it is indeed one of those goals.