When is there a case for refactoring large parts of a codebase at once? (if ever)

Question

I work with a specific engineer (let's call them X) that, even though they have many years of experience in the industry, has a lot of trouble to manage the scope of code changes. For example, say that we as a team have estimated that a task will take '5' story points, X will claim that all existing code is 'bad' - and should be refactored immediately. In the past this has led to enormous amounts of changes, which in turn lower our team velocity and introduce new bugs (we have low test coverage).

I think this engineer has a classic case of 'all legacy code is bad and the only way forward is to refactor it all'-thinking; but X doesn't introduce new tests to verify the functionality.

When I personally make changes in code and I notice that the code is of insufficient quality, I will make an extra effort to improve it - but only piece by piece - so that the scope doesn't grow too much and we reduce the risk of new changes.

My question is: when is it okay to do a big refactor in order to achieve a small task? Is it wrong to state that any improvements can be done incrementally, and should be?

• Flater: The correctness an answer hinges on specifics that this question lacks. There are cases where refactoring is a pressing concern (e.g. when it bogs down every new change/bug task or new changes constantly introduce bugs), but there are also cases where refactoring is being done overzealously and the effort invested does not pay back any meaningful dividends. This depends on both the current state and future expectations of the project.
• Michael Kohne: There are no hard and fast rules on this subject. It's entirely a matter of experience both of the developer and with the codebase in question. (As @Flater notes, some codebases are worth the refactor). This honestly sounds more like a management problem than anything else. Perhaps rules about test code would help (either you'd get tested code, OR the dev would reduce change scope to avoid writing it).
• gnat: Does this answer your question? When is a BIG Rewrite the answer?
• gnasher729: To answer the question only: There is a case when there is someone foolish enough to pay for it, you enjoy refactoring, and the fun and the money is more important to you than the results.

Answer 1

Big and especially non-local refactorings should be a job on their own, with all that implies about planning and being clear upfront what the plan is and how long it will take.

And as you've observed, refactoring without adequate test coverage is performing without a safety net.

Some of this depends what language you're operating in. I feel much more confident about big refactorings in a strongly-typed language with good tools for it; C#+Resharper for example can automate tasks such as "move method from one class to another and update all calls".

Ultimately I think you have to press them for detail. Don't let people embark on vague "refactoring" to suit their taste. Insist they lay out a plan and make a case for how it will improve future work. Make them compare this in public with the "just do it" approach.

Answer 2

The general recommended approach to refactoring is to first write sufficient tests so you have good understanding how the current system works. Then you can start moving functionality to to new, better designed, modules, rewriting it in the processes. Eventually all the important code should be in well designed modules, and you can clear out any remaining old 'glue'-code.

That said, how to do proper refactoring or rewrites is a very complex subject that will be very dependent on the exact code base and team.

I would suggest to introduce a code review and testing process if this is not already in place. If the developer is refactoring large sections of code, it would presumably be more readable and easier to understand. A code review should tell if this is actually the case, or if the developer just prefers to do a rewrite rather than trying to understand how the current code works. Testing, automated or otherwise, should help discover any regressions.

If readability is actually significantly improved, a somewhat lower velocity and higher defect count might be acceptable, since this would amount to paying down technical debt. But if none of the metrics are better you should discuss in the team about the best way to approach problems and how to improve your velocity.

• Greg Burghardt: I like this answer, except for accepting a higher defect count. Improved code should have fewer defects, otherwise developers just write more legacy code.
• JonasH: @greg Burghardt I would argue new code tend to have bugs. If you are doing a major refactoring you would write more new code, so more chances for bugs. But there might still be good reasons to said refactoring, it might fix more serious bugs, be required for needed features, improve overall architecture and readability etc.