The High-Risk Refactoring(webup.org)
webup.org
The High-Risk Refactoring
https://webup.org/blog/the-high-risk-refactoring/
44 comments
That strategy is something the heart of "agile development." Ensure you can do a little bit and then walk away and work on something else while still having delivered some value.
A crux though for "big refactoring project" - first, that is kinda misnomer. It's not refactoring, it's re-architecture. Second, a number of refactorings can make things worse before they are better. For example, fixing bad abstractions, sometimes the first step is to inline stuff and copy/paste the abstractions to first "flatten the code" before restructuring it. That re-flattening step can lead to worse code, but it's in a place where it can be then made better (one step back to enable three steps forward)
A crux though for "big refactoring project" - first, that is kinda misnomer. It's not refactoring, it's re-architecture. Second, a number of refactorings can make things worse before they are better. For example, fixing bad abstractions, sometimes the first step is to inline stuff and copy/paste the abstractions to first "flatten the code" before restructuring it. That re-flattening step can lead to worse code, but it's in a place where it can be then made better (one step back to enable three steps forward)
> It's not refactoring, it's re-architecture.
If you change the code without changing the functionality, it's a refactoring.
If you change the main organization of the code, it's re-architecturing.
It being one of those has no impact on whether it's also the other.
If you change the code without changing the functionality, it's a refactoring.
If you change the main organization of the code, it's re-architecturing.
It being one of those has no impact on whether it's also the other.
I ran across this paper recently which conveys it better than I can:
"[refactoring], defined traditionally as a set of program transformations intended to improve the system design while preserving the behavior. Refactoring studies are expanded beyond code-level restructuring to be applied at different levels (architecture, model, requirements, etc.), adopted in many domains beyond the object-oriented paradigm (cloud computing, mobile, web, etc.), used in industrial settings and considered objectives beyond improving the design to include other non-functional requirements (e.g., improve performance, security, etc.)." [1]
I've been having a lot of thoughts lately that we should stop calling things 'refactoring'. There is no incentive to do refactoring for anyone other than a developer (users don't want it, PM do not want it, QA do not want it, peer Dev's do not want to review it). Instead, we should probably be more direct in the language, "this was an update to existing module X so that it would be consistent with the updates in module Y. The two modules were previously consistent, this is an update to maintain consistency." Rather than "Refactored module X". That latter phrasing invites the very leading question: "you are working on Y, why did you touch X?"
At the end of the day, the ideas, tools & techniques are useful, the vocabulary can get in the way. (I am guilty of letting that vocabulary get in the way. At the same time, it is troubling. How are we to share ideas without the starting point of a shared vocabulary?)
[1] https://deepblue.lib.umich.edu/bitstream/handle/2027.42/1558...
"[refactoring], defined traditionally as a set of program transformations intended to improve the system design while preserving the behavior. Refactoring studies are expanded beyond code-level restructuring to be applied at different levels (architecture, model, requirements, etc.), adopted in many domains beyond the object-oriented paradigm (cloud computing, mobile, web, etc.), used in industrial settings and considered objectives beyond improving the design to include other non-functional requirements (e.g., improve performance, security, etc.)." [1]
I've been having a lot of thoughts lately that we should stop calling things 'refactoring'. There is no incentive to do refactoring for anyone other than a developer (users don't want it, PM do not want it, QA do not want it, peer Dev's do not want to review it). Instead, we should probably be more direct in the language, "this was an update to existing module X so that it would be consistent with the updates in module Y. The two modules were previously consistent, this is an update to maintain consistency." Rather than "Refactored module X". That latter phrasing invites the very leading question: "you are working on Y, why did you touch X?"
At the end of the day, the ideas, tools & techniques are useful, the vocabulary can get in the way. (I am guilty of letting that vocabulary get in the way. At the same time, it is troubling. How are we to share ideas without the starting point of a shared vocabulary?)
[1] https://deepblue.lib.umich.edu/bitstream/handle/2027.42/1558...
[deleted]
100% my thought ;p
And agile can absolutely suck for refactors, because it only rewards superficial steps.
I generally agree, but with the caveat that you are more speaking to "Agile" and agile project management (eg: Scrum).
To that extent, I find it really useful to draw a distinction between agile development and agile project management. The former I think has some good ideas, namely :
- don't go off into a cave for days, weeks or even many months before merging or showing any work; show & release your work often.
- ask users frequently: "here this is, what do you think?"
- assume you are probably on the wrong track, a good way to get back on track is to talk to the users. The more often you talk to users, the less likely you are to spend time on the wrong track
- deliver in small increments. Gall's law - don't do big-bang deliver-the-world style projects, they often fail.
On the other hand, agile project management (IMO) is almost trivial when agile development is done well and should not be prescriptive. If someone is building a tool for someone sitting next to them, does it make sense to do a demo every two weeks? I find what's more that Scrum is not even coherent, it's not internally consistent with itself. It's ironic as Scrum says nothing about going "fast", yet it's all about "fast" development - and second a framework that says "we will be changing our mind every two weeks" is somehow supposed to be more predictable and a better fit for long-range project timelines??
ON THE OTHER HAND... in the defense of Scrum, having a framework to prioritize work and emphasize delivery can still be valuable. Dev's can have a tendency to only want to work on cleaning up code. It can be a bit like Starcraft, where a player spends too long building economy and workers and never actually builds the army that wins the game. Though, frameworks have a bit too often become shorthand for actual thought, too much cargo-culting.
[edits: cut out some ranting]
To that extent, I find it really useful to draw a distinction between agile development and agile project management. The former I think has some good ideas, namely :
- don't go off into a cave for days, weeks or even many months before merging or showing any work; show & release your work often.
- ask users frequently: "here this is, what do you think?"
- assume you are probably on the wrong track, a good way to get back on track is to talk to the users. The more often you talk to users, the less likely you are to spend time on the wrong track
- deliver in small increments. Gall's law - don't do big-bang deliver-the-world style projects, they often fail.
On the other hand, agile project management (IMO) is almost trivial when agile development is done well and should not be prescriptive. If someone is building a tool for someone sitting next to them, does it make sense to do a demo every two weeks? I find what's more that Scrum is not even coherent, it's not internally consistent with itself. It's ironic as Scrum says nothing about going "fast", yet it's all about "fast" development - and second a framework that says "we will be changing our mind every two weeks" is somehow supposed to be more predictable and a better fit for long-range project timelines??
ON THE OTHER HAND... in the defense of Scrum, having a framework to prioritize work and emphasize delivery can still be valuable. Dev's can have a tendency to only want to work on cleaning up code. It can be a bit like Starcraft, where a player spends too long building economy and workers and never actually builds the army that wins the game. Though, frameworks have a bit too often become shorthand for actual thought, too much cargo-culting.
[edits: cut out some ranting]
Agile is fundamentally a "UI convergence" technique. It is perfect for interacting visually and conceptualizing requirements (and the realization of those requirements) with end users and other stakeholders.
I'm not saying waterfall is the only alternative. But one week sprints is a dumb idea for any serious software development.
The other thing Agile does is give too much priority to outside concerns. Like pulling people off to do some thing the support team should fundamentally own, or meetings, support tickets that get too high prioritization because agile gives them too much prioritization.
Sometimes you have to sit in an office undisturbed. To Agile people, that's the worst thing ever, but fundamentally everything truly significant in software stemmed from that.
I'm not saying waterfall is the only alternative. But one week sprints is a dumb idea for any serious software development.
The other thing Agile does is give too much priority to outside concerns. Like pulling people off to do some thing the support team should fundamentally own, or meetings, support tickets that get too high prioritization because agile gives them too much prioritization.
Sometimes you have to sit in an office undisturbed. To Agile people, that's the worst thing ever, but fundamentally everything truly significant in software stemmed from that.
Interesting: > I'm not saying waterfall is the only alternative. But one week sprints is a dumb idea for any serious software development.
I just proposed to our team we move from 3 week sprints to 1 week. It's working out far better already. In part it's already waterfall done as sprints, so the duration is not as bad, but it's helping my boss stay a lot more focused on delivering and keeping in mind WIP limits.
I do draw a distinction on "Agile" between 'agile development' and 'agile project management'. Unqualified, I would assume "Agile" to mean "agile project management", in which case IMHO it's mostly garbage. The interesting stuff is 'agile development' IMO, the project management is context dependent and would tend to flow readily from a development project where:
- developers focus on releasing early & often
- developers are speaking directly to customers frequently
- customers/users are interacting with the software as it is being built and are providing feedback along the way
> Sometimes you have to sit in an office undisturbed. To Agile people, that's the worst thing ever, but fundamentally everything truly significant in software stemmed from that.
I would tend to agree. My impression is generic IT management, or business & project management stems from just the 1920s & has evolved only so much. There's a lot of emphasis on predictability, removing variation, reducing risk and maximizing utilization. (eg: Gantt [1], and there is someone else whose name I can't think of around the same time that was also as foundational). For the context in which that management was created, in part it's fine (ableit de-humanizing) - but for an industry that is more akin to a story writer than a factory worker - it's not really fine.
[1] https://en.wikipedia.org/wiki/Henry_Gantt
I just proposed to our team we move from 3 week sprints to 1 week. It's working out far better already. In part it's already waterfall done as sprints, so the duration is not as bad, but it's helping my boss stay a lot more focused on delivering and keeping in mind WIP limits.
I do draw a distinction on "Agile" between 'agile development' and 'agile project management'. Unqualified, I would assume "Agile" to mean "agile project management", in which case IMHO it's mostly garbage. The interesting stuff is 'agile development' IMO, the project management is context dependent and would tend to flow readily from a development project where:
- developers focus on releasing early & often
- developers are speaking directly to customers frequently
- customers/users are interacting with the software as it is being built and are providing feedback along the way
> Sometimes you have to sit in an office undisturbed. To Agile people, that's the worst thing ever, but fundamentally everything truly significant in software stemmed from that.
I would tend to agree. My impression is generic IT management, or business & project management stems from just the 1920s & has evolved only so much. There's a lot of emphasis on predictability, removing variation, reducing risk and maximizing utilization. (eg: Gantt [1], and there is someone else whose name I can't think of around the same time that was also as foundational). For the context in which that management was created, in part it's fine (ableit de-humanizing) - but for an industry that is more akin to a story writer than a factory worker - it's not really fine.
[1] https://en.wikipedia.org/wiki/Henry_Gantt
That should be done on a branch and not merged to the trunk until there is a net improvement.
Maybe, though as a hard and fast rule I strongly disagree.
Delaying merge increases risk. It can also make the diff impossible to review. Overhead can be increased by way of merge conflict and re-doubling of efforts that would not be necessary had the update been merged.
Other times, sometimes you want the one step to be launched so you can ensure there is no regression. Sometimes the next steps are going to take a long time and themselves are multi-pronged and would be merged in multiple parallel phases.
Sometimes you just want to get rid of the inappropriate abstraction so that someone can complete task X where inappopriate abstraction was hindering that. In other times, "worse" is in the eye of the beholder, WET'ing the code first can sometimes be the right thing to do, even if it is never re-DRY'ied.
Delaying merge increases risk. It can also make the diff impossible to review. Overhead can be increased by way of merge conflict and re-doubling of efforts that would not be necessary had the update been merged.
Other times, sometimes you want the one step to be launched so you can ensure there is no regression. Sometimes the next steps are going to take a long time and themselves are multi-pronged and would be merged in multiple parallel phases.
Sometimes you just want to get rid of the inappropriate abstraction so that someone can complete task X where inappopriate abstraction was hindering that. In other times, "worse" is in the eye of the beholder, WET'ing the code first can sometimes be the right thing to do, even if it is never re-DRY'ied.
One more example, the phrase "consistently bad is better than inconsistently good"
Sometimes you have 10 modules that all need the same update to be better. Updating just one module, while that module might be a lot better - breaks consistency and makes the codebase harder to understand overall. It is worse, even though that one module is better. Though, updating all 10 modules at the same time might be just impractical and super risky. It can make sense to do that in phases and do just one at a time. Sometimes the effort for the one module will take a few weeks. Holding all that up for most of a year does not make a lot of sense.
At the same time, trying to do it all across the whole code-base all at once, at some point that is no longer even refactoring:
"Refactoring isn't a special task that would show up in a project plan. Done well, it's a regular part of programming activity. When I need to add a new feature to a codebase, I look at the existing code and consider whether it's structured in such a way to make the new change straightforward. If it isn't, then I refactor the existing code to make this new addition easy. By refactoring first in this way, I usually find it's faster than if I hadn't carried out the refactoring first." (https://refactoring.com/)
Of note, sometimes that initial refactoring should land as its own change first. Though, that is a philosophical disagreement that is okay to have. Some reviewers want you to fix up existing problems in code you touched, and they want that in the same diff as the bug fix and/or feature update. Other reviewers want those updates to be completely separate and done one-by-one.
Sometimes you have 10 modules that all need the same update to be better. Updating just one module, while that module might be a lot better - breaks consistency and makes the codebase harder to understand overall. It is worse, even though that one module is better. Though, updating all 10 modules at the same time might be just impractical and super risky. It can make sense to do that in phases and do just one at a time. Sometimes the effort for the one module will take a few weeks. Holding all that up for most of a year does not make a lot of sense.
At the same time, trying to do it all across the whole code-base all at once, at some point that is no longer even refactoring:
"Refactoring isn't a special task that would show up in a project plan. Done well, it's a regular part of programming activity. When I need to add a new feature to a codebase, I look at the existing code and consider whether it's structured in such a way to make the new change straightforward. If it isn't, then I refactor the existing code to make this new addition easy. By refactoring first in this way, I usually find it's faster than if I hadn't carried out the refactoring first." (https://refactoring.com/)
Of note, sometimes that initial refactoring should land as its own change first. Though, that is a philosophical disagreement that is okay to have. Some reviewers want you to fix up existing problems in code you touched, and they want that in the same diff as the bug fix and/or feature update. Other reviewers want those updates to be completely separate and done one-by-one.
I like this take a lot.
At my work we often do a lot of “pre-commits” for projects that require any significant amount of work. A pre-commit is usually pure moving/repackaging of code or small re-writes to how we do things now to make the PR (or PRs) for the actual feature the most straightforward and pure business-logic-only changes that we can. So, I usually tell people we rarely refactor, but maybe we do kinda what you’re recommending here.
At my work we often do a lot of “pre-commits” for projects that require any significant amount of work. A pre-commit is usually pure moving/repackaging of code or small re-writes to how we do things now to make the PR (or PRs) for the actual feature the most straightforward and pure business-logic-only changes that we can. So, I usually tell people we rarely refactor, but maybe we do kinda what you’re recommending here.
“Make the change easy, then make the easy change”
Which makes cancelling the project an easier decision.
In the beginning of my career I was only accepting short term projects, so I learned to always leave the code in a state where anybody can pick it up.
I still code in that way, but I know it makes me more replaceable.
If anyone at any company judges an engineer who crafts code that is easy for other engineers on their team to immediately pick up and start work on as more replaceable, that person is hopelessly incompetent. That would normally be a basic skill every engineer is expected to pick up before getting too far in their career.
If anything, an engineer with a clear tendency to try to write themselves job security is the one that should be replaced ASAP.
If anything, an engineer with a clear tendency to try to write themselves job security is the one that should be replaced ASAP.
I would immediately dismiss an engineer who codes with the goal of "being unreplaceable". That's a nightmare scenario. Our job is to leave the code as ready to hand off as possible. Engineers who do it well are given bigger and better things.
Your friend is doing an actual refactoring (kudos), according to the definition. Otherwise it's more like restructuring
> works with the assumption that the project could get cancelled at any moment
If I have that feeling, I have very, very little incentive to contribute to the project until I get better clarity on whether leadership wants to continue supporting the project and see it to launch.
I like working on tech because I want to see it launched or at LEAST open-sourced if not launched, not die in a NDA graveyard.
If I have that feeling, I have very, very little incentive to contribute to the project until I get better clarity on whether leadership wants to continue supporting the project and see it to launch.
I like working on tech because I want to see it launched or at LEAST open-sourced if not launched, not die in a NDA graveyard.
I love this thanks for sharing this
how does that assumption work? I would have thought that assumption would lead one to care less.
It works if simonw means the refactoring/rearchitecting/redesign project and not the broader encompassing project. In the former case, it means making sure your changes are mergeable (ideally merged) all along the way so that if that effort is canceled the broader project can still benefit from it. In the latter case, then you're correct it doesn't matter. If the encompassing project is canceled who cares what state the change effort is in since everything is getting tossed.
What I did last time for a moderate refactoring was to do everything step by step in the commits. Intermediate broken state was fine. Them purposefully commit in such a way that the tools could verify that I did the intended change. Like line moves: verify with `git diff --color-moved`. Then I told the reviewers about my breadcrumbs. Then finally for the merge I could rewrite the history.
Ideally I want to not just post a 500-line diff as a pull request. Ideally I want to really write a program/script which does as much of the changes as I did (intermediate commits for the changes that need to be done manually). Refactorings are often done through and IDE so hopefully the IDE can output a script of the refactoring steps. Then the reviewers don’t even have to look at the diff (really): they can look at the script, see if it is reasonable, then run it themselves and verify that it produces the same output (the same tree).
Coccinelle is a tool for C (and C++?) which lets you write “semantic patches” like “replace these boolean expressions with this one”. Maybe replace some considered-bad C standard library calls with what you consider to be better. Then you can leave those patches in and check that people don’t check in new code with the old pattern.
All of this is just for refactoring though. Once you start talking about rewriting I feel like you have moved beyond that point.
Ideally I want to not just post a 500-line diff as a pull request. Ideally I want to really write a program/script which does as much of the changes as I did (intermediate commits for the changes that need to be done manually). Refactorings are often done through and IDE so hopefully the IDE can output a script of the refactoring steps. Then the reviewers don’t even have to look at the diff (really): they can look at the script, see if it is reasonable, then run it themselves and verify that it produces the same output (the same tree).
Coccinelle is a tool for C (and C++?) which lets you write “semantic patches” like “replace these boolean expressions with this one”. Maybe replace some considered-bad C standard library calls with what you consider to be better. Then you can leave those patches in and check that people don’t check in new code with the old pattern.
All of this is just for refactoring though. Once you start talking about rewriting I feel like you have moved beyond that point.
Refactoring and related maintenance activities, like library/framework upgrades, are high risk and low immediate reward. In my experience, they're also one of the hardest overall software development tasks; large-scale refactors will end up touching a large percentage of existing code, and will require a ton of reading/understanding existing code to figure out what it does, as well as the domain knowledge to know what it should do. Often during refactoring I've found hairy bits of code which seem like they'll be intractable, until I stepped up a level and realized it's for a half-implemented feature that never got released, and can be completely removed.
If you try to measure it on an incremental level, refactoring is never worth it. Your code changes can and will cause unexpected bugs; customers get no perceivable value; it's engineering time not spent on revenue-generating features, and per the previous paragraph, you should be getting your best engineers doing large refactors. But if you look at it across a longer horizon, it makes a massive difference in the maintainability of your codebase. It's like cutting out 300 calories/day from your diet; you won't notice a difference if you weigh yourself on two consecutive days, but if you do it for 6 months it has a noticeable impact.
If you try to measure it on an incremental level, refactoring is never worth it. Your code changes can and will cause unexpected bugs; customers get no perceivable value; it's engineering time not spent on revenue-generating features, and per the previous paragraph, you should be getting your best engineers doing large refactors. But if you look at it across a longer horizon, it makes a massive difference in the maintainability of your codebase. It's like cutting out 300 calories/day from your diet; you won't notice a difference if you weigh yourself on two consecutive days, but if you do it for 6 months it has a noticeable impact.
The way to make refactoring safe is to have a solid test suite.
With a good enough test suite, if all tests still pass, you can be pretty confident.
To me, making refactoring easier/safer is probably the biggest benefit of having tests!
With a good enough test suite, if all tests still pass, you can be pretty confident.
To me, making refactoring easier/safer is probably the biggest benefit of having tests!
I'd say making adding new features that deliver value safer is an even bigger benefit of having tests.
I have yet to work anywhere where people refactor anywhere near as much as they should, so I think this is really sending the wrong message. People massively overestimate the risks of refactoring and massively underestimate the risks of not refactoring - which many people incorrectly assume don't exist.
This is my viewpoint too, but I'm always wary of cargo cult refactors. I've seen multiple cases where a refactor moves the needle laterally (or even down).
I also feel similarly about rewrites. I think people are way too scared of them, but unfortunately I've also seen and heard of cases of bad rewrites. This makes it much harder to convince someone that a rewrite is worth it.
In essence, both refactors and rewrites can have great outcomes if the reasoning for it is sound.
I also feel similarly about rewrites. I think people are way too scared of them, but unfortunately I've also seen and heard of cases of bad rewrites. This makes it much harder to convince someone that a rewrite is worth it.
In essence, both refactors and rewrites can have great outcomes if the reasoning for it is sound.
For me, refactoring means simple, no-risk code re-organization only. It should be executed often during the production and it should serve foremost code-reuse and better readability. Anything going further than that is for me rewriting and should be considered appropriately.
Indeed. Re-writing is often dangerous: https://www.joelonsoftware.com/2000/04/06/things-you-should-...
You are absolutely right, see the definition on https://refactoring.com/
Unfortunately, it has become common to refer to any kind of code changes as refactorings (not backed by any proof, just an observation)
One approach I've made work a couple of times is to pull all of the data going in and out of a system for a time period, and rerun it after the refactor. This gave me a lot more confidence than unit tests.
One of the subtler but quite dangerous risks of refactoring, is making the output more “correct” but different from what it was.
Clients of the application likely have adapted to the less correct output in ways that will break in unanticipated ways if it changes. I’ve seen this lead to production issues, and the explanation that the new behavior was more correct did not go over well.
Clients of the application likely have adapted to the less correct output in ways that will break in unanticipated ways if it changes. I’ve seen this lead to production issues, and the explanation that the new behavior was more correct did not go over well.
Refactoring code is the same as refactoring a math equation. Make sure you show every step so you don't screw up. If you skip steps you will screw it up.
If it's risky, you're doing it wrong. (Or you're using an unchecked language like Python (which is incidentally why Python is not suited for large projects IMO, because you can't safely refactor them).)
P.S. the steps are covered in Martin Fowler's book, but they are equivalent to refactoring functions in math, naturally.
If it's risky, you're doing it wrong. (Or you're using an unchecked language like Python (which is incidentally why Python is not suited for large projects IMO, because you can't safely refactor them).)
P.S. the steps are covered in Martin Fowler's book, but they are equivalent to refactoring functions in math, naturally.
I think you mean "untyped", which is only as true as you make it. Type annotations have been mainstream for many years now.
I am surprised at the tone here as well as that if the article.
If you view refactoring as this risky, you are I imagine using a dynamically typed language with automated tests few and far between.
In a statically typed language and with a decent automated test suite, refactoring is just another coding activity.
It sounds like people out there are terrified of going near their code with a 10’ pole. That is heart breaking to see.
If you view refactoring as this risky, you are I imagine using a dynamically typed language with automated tests few and far between.
In a statically typed language and with a decent automated test suite, refactoring is just another coding activity.
It sounds like people out there are terrified of going near their code with a 10’ pole. That is heart breaking to see.
Maybe don’t assume other people are “wrong”? A strongly typed codebase in a larger system almost certainly has untyped boundaries (HTTP, JSON, SQL, etc.) and likely to data driven with combinatorial complexity. Going into such rewrites with a great deal of respect for any existing code that “works” is very wise.
I guess we should all stop fixing bugs and creating new feature and enhancements.
My God - if we touch the code, something might break!
Sorry for the sarcasm, but this attitude of fear is so pervasive and unnecessary.
Seriously, isn’t this our job?
My God - if we touch the code, something might break!
Sorry for the sarcasm, but this attitude of fear is so pervasive and unnecessary.
Seriously, isn’t this our job?
Respect, not fear. I chose the word intentionally. Respecting the working system means investing a little before going in to redo. That’s all.
No everyone is right for such work, which is fine because they are often well suited for other equally important things.
No everyone is right for such work, which is fine because they are often well suited for other equally important things.
I start with questions like -
- What are the engineering and business benefits of refactoring the current code base?
- What is the cost of not refactoring code base? Does it affect user experience, performance of the app?
- Does it significantly improves onboarding time for new engineers?
- Does it enable the team to incorporate future updates or feature development?
etc.
Any engineering tech-debt has to be justified with outcomes, and needs to be weighed against current priorities. Else it will be a waste of critical resource.
Good reminder to try and improve things before one gets to the point where a large refactor is needed. Not always avoidable of course.
Just a reminder that you won't have to refactor if you don't factor it too much in the first place. It ought to be easier to conceive, understand, program, and maintain. If you have to refactor, you probably have to defactor instead.
I like this idea. What does it mean though? Does it mean proper enforcement of isolation of concerns and increased modularity?
I think this is a really smart strategy.