You’ve decided to clean up some of your code, the reason being, “reducing technical debt”. So, the code is chosen based on the subjective feeling of how “messy” it is. Now it’s time for refactoring: you move some functionality here and there, rename a couple of variables, and simplify a pair of methods. You’re committing the changes in VCS and move on to the next task in a completely unrelated code. Maybe in some future we’re gonna make actual user-visible changes related to the refactored code.
Sounds like a usual thing which is good, right?
I say — not really.
While intention for doing such refactorings is noble, let’s look at this situation one more time — we’ve changed the code and this change:
a) Didn’t result in any user-visible changes
b) Didn’t contribute to the process of making some actual user-visible change because we’ve moved to another task in adifferent place
So again, was it a really good call?
And I argue that not only didn’t it give us anything to make life of our users and ourselves easier, but events could make it worse.
And before I’ll elaborate here what I mean by user-visible change (e.g. not refactoring).
- Fixed bug
- New feature
- Performance optimisation
- Usability improvement
- …basically any change the code, after which the application supposed to behave differently for the end user
Saying no to “refactoring for the sake of refactoring”
Now, the arguments. There are just 2 actually:
We’ve might make things worse for our users
Refactoring, by nature, is just another change in the code. So, maybe we’ll introduce a regression by it. If we pass the refactoring to the code review and QA, we’re spending the team’s efforts on something that didn’t bring value for our users.
We’ve might make things worse for our ourselves
Paradoxically, by performing refactoring in a separate timeline with actual changes for which refactoring is meant to be performed we may produce even messier code, including additional VCS history modifications.
For instance, we might assume that extracting common functionality for later use may be a good idea. We take a look at existing usages and make an assumption about what future usages will look like. So we’re doing this refactoring just to be later realised that following usages could be different.
At the same time, the best possible outcome of this activity is “nothing bad happened, few”. Was it worth it?
So, we should not do refactoring at all?
Not at all! We need to do refactoring, constantly actually. Otherwise we’ll experience various aspects of downsides: From the very concrete like slowing down our throughput and quality of the code to subtle ones like actual joy of working with the code. But how do we do it?
Simple. Every refactoring should be a part of user-visible change you’re working on right now. You do it only “right before” the actual change that was meant to be made.
If you’re planning to extract common code for future usages — make it right before introducing this new usage. You’ll have a clear picture of what common code should look like — you have an intended use case right here!
Algorithm simplification? Only if you’re gonna make actual changes there as well. QA will thank you for being able to clearly see what to test because you’ll have a proper issue in your issue tracker.
Some renaming for better code understanding when you read it? OK, but why do you read it? If you’re gonna use it for new functionality — great, that’s the best time to rename. If you’re just reading it for your own curiosity — please, let it be. Your colleagues will thank you for keeping VCS history clean and not introducing regressions by some changes.
Also, efforts saved on not reviewing this refactoring (because it’ll not take place) might be spent on the thing for which we’re all here — to improve our actual applications behaviours, making life of the users better.
