Our take on opportunistic refactoring
Code base is somewhat of a changing substance. Software product itself is not like any other product. Unlike building that also has architecture, a piece of software doesn’t stay there unchanged for ages once built. And unlike, say, gadgets, there is no new release for a piece of software after which you should trash your old one.
No, software evolves instead. And it’s totally normal that during this evolution some architectural, structural or other decisions don’t fit as good as they did earlier.
At some point your initial decisions start to act against you when new feature requests come in. And that’s the point where you make a choice: to do it right or to do it fast and deal with technical debt later on. Most of the time developers choose wrong. And that’s how a legacy code is born.
As project development goes on, more decisions are made in favor of fast. Until finally developer is forced to choose right simply because fast is not fast anymore.
When the amount of technical dept is overwhelming — teams start to think about refactoring to adjust application architecture with new requirements. At this point full-blown refactoring is a very risky business. And necessity of the refactoring is obvious to the development team only. Customer or PO wants new features to be delivered at the same pace as previously, and it’s not that easy to explain to him, why the feature development should freeze for a couple of weeks, which not only won’t bring any new business value, but may also introduce new bags.
Sounds familiar?
Luckily, there is an approach that lies somewhere between fast feature delivery and full-blown refactoring. It’s called opportunistic refactoring and is based on the boy scout rule:
Leave your code better than you found it.
It means cleaning up the code piece by piece in places that are affected by a new feature implementation if you have an opportunity.
You may get something working, but realize that it would be better if the interaction with existing classes was changed. Take that opportunity to do that before you consider yourself done. — Martin Fowler, Opportunistic refactoring
All this sounds simple enough and wise. But incorporating it in a legacy project requires a very dedicated team. Every team member should clearly understand why he has to do more implementing some simple feature. Every team member should understand how current state of the project could be improved and what he can do to improve it during implementation.
That’s where we’ve come up with the approach that’s successfully used in our team for some very nasty legacy projects.
In order to establish this boy scout attitude, we’ve extended a little our code review process.
It basically goes as follows: if during feature implementation you’ve introduced some changes to the class, this full class is now eligible for the code review.
It could be started gradually, in a form of suggestions first. But after a while you will notice that team members do more already, anticipating those suggestions during the review process.
Moreover, the reviewer shouldn’t always be a passive figure in this process. He can also push some changes for the reverse review.
As long as it works in your team.
This take on opportunistic refactoring helps with the following aspects:
- there is almost no synchronization required between team members: once some feature is considered to be an independent piece of work — all surrounding refactoring is also more or less independent;
- work becomes more transparent to the customer and brings less risks than dedicated refactor-everything process;
- features are delivered along with some code improvements;
- rite of passage goes more natural with the use of known processes like a code review; no one is forced to suddenly become a boy scout.
Surprisingly, I found out that this approach can meet a heavy friction in some teams. And instead of embracing a practice, where a motivated team member does more than asked, it’s turned down by team members and management. That is very improvident.
Here are some suggestions how to deal with it:
- in case there is some king of rejection of this approach among team members — start establishing it only with the ones that are interested; it could be spread across the team member by member;
- in case there is any kind of concern from management side — don’t tell your manager what exactly you are doing; features are delivered and planning is never precise; after all — code review is your internal team business.
Also in answers to this StackExchange thread, a lot of people suggest to split feature implementation and refactoring somehow (different PR, different commits) to make changes clear to the reviewer. But I am against it. Since refactoring was induced by the feature implementation — both should be treated as a part of one change.
Overall, it’s a good practice to do more for every change, keep code base clean and design decisions clear.
With this approach it’s also easy to notice that there are no routine tasks. Every task can add up not only business value, but also value in terms of code quality.
