The High-Risk Refactoring
webup.org
webup.org
I think this is a really smart strategy.
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)
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.
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.
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.
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.
"[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...
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]
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 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.
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 anything, an engineer with a clear tendency to try to write themselves job security is the one that should be replaced ASAP.
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.
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.
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 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.
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.
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 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.
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?
No everyone is right for such work, which is fine because they are often well suited for other equally important things.