The High-Risk Refactoring

https://webup.org/blog/the-high-risk-refactoring/

by ben_s • 2 years ago

71 43 2 years ago

simonw• 2 years ago

A friend once told me that any time he takes on a big refactoring project at work he works with the assumption that the project could get cancelled at any moment, so his goal is to ensure that if it DOES get cancelled he'll still be leaving the code in a better place than it was when he started.

I think this is a really smart strategy.

bigkm• 2 years ago

how does that assumption work? I would have thought that assumption would lead one to care less.

Jtsummers• 2 years ago

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.

seadan836• 2 years ago

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)

nradov• 2 years ago

That should be done on a branch and not merged to the trunk until there is a net improvement.

marcosdumay• 2 years ago

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.

AtlasBarfed2• 2 years ago

And agile can absolutely suck for refactors, because it only rewards superficial steps.

dheera• 2 years ago

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.

latenightcoding• 2 years ago

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.

jrajav• 2 years ago

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.

JackMorgan• 2 years ago

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.

0xferruccio• 2 years ago

I love this thanks for sharing this

Fire-Dragon-DoL• 2 years ago

Your friend is doing an actual refactoring (kudos), according to the definition. Otherwise it's more like restructuring

sinuhe69• 2 years ago

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.

ck45• 2 years ago

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)

begueradj• 2 years ago

Indeed. Re-writing is often dangerous: https://www.joelonsoftware.com/2000/04/06/things-you-should-...

IshKebab• 2 years ago

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.

dimgl2• 2 years ago

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.

mjr00• 2 years ago

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.

BurningFrog• 2 years ago

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!

zenogantner• 2 years ago

I'd say making adding new features that deliver value safer is an even bigger benefit of having tests.

keybored• 2 years ago

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.

jimbokun• 2 years ago

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.

Ferret7446• 2 years ago

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.

computerfriend• 2 years ago

I think you mean "untyped", which is only as true as you make it. Type annotations have been mainstream for many years now.

Scubabear682• 2 years ago

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.

mlhpdx2• 2 years ago

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.

Scubabear682• 2 years ago

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?

sweezyjeezy• 2 years ago

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.

wruza• 2 years ago

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.

dimgl2• 2 years ago

I like this idea. What does it mean though? Does it mean proper enforcement of isolation of concerns and increased modularity?

gopal_virtual• 2 years ago

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.

liampulles• 2 years ago

Good reminder to try and improve things before one gets to the point where a large refactor is needed. Not always avoidable of course.

The High-Risk Refactoring

Related Stories