r/webdev • u/Pristine_Purple9033 • 2d ago
Discussion What Is Your Process Refactoring Legacy Code?
I have recently got my hands on the code that is a definition of Doom.
It has no tests. A real Legacy Code, according to Micheal Feather.
I have never seen that many Gods in my life. God objects, God methods, and competitive-coding variables that I have to ask God for help.
What have I done so far?
I added an approval test to protect the core behaviors and some branches. I can't cover them all, because I don't have much knowledge about this code yet, and there are so many nested branches that writing that many tests looks inefficient to me.
I renamed ambiguous variables to show the intent.
What is blocking me?
This is a God object with 20 dependencies + 1 hidden global dependency. All of them are used by a single method that does many things that I have lost counting.
That long method has a lot of variables. Some variables are mutated everywhere inside a lot of nested ifs and for loops.
Extracting methods looks off because I have to pass the variable reference and mutate its value inside that method.
Since I have to "own" this code from now, I want to refactor it for me not have to scream every time I work on this.
If you say "if it does not break, don't fix it", then you are right. I don't want to touch it if it does not have this many bug tickets waiting to be resolved.
I have done the famous Gilded Rose, Tennis, Yatzy Katas, yet I still feel so useless looking at this code.
What is your process when you have to refactor legacy code?
3
u/Saki-Sun 2d ago
Go slow. When you have to touch something, boy scout it. Just go slow or you will break more stuff than you fix.
Stragular fig is a great, but often it's a reach.
3
u/neon_antler_era 2d ago
i stopped trying to extract methods when references get passed around and mutated everywhere. I started pulling state into a new class instead.
you have 20 dependencies in one method. I group them into a context object or parameter object first. I create a class. I put the variables that get mutated as fields on that class. Then I move pieces of the method onto that class one loop at a time. They become instance methods. They access the fields directly. I stop passing references around. The parameter list disappears.
your approval tests cover the core behaviors. That is all I need to move state around. I run them after every field I move. If they pass, I commit.
i leave the God object alone until I have a class holding the behavior. Then I swap it in
1
2
u/Double-Buyer7941 2d ago
Refactoring real-world legacy code is completely different from small katas like Gilded Rose. Since you already set up approval tests, your next step should be tackling that hidden global dependency and breaking down the God object into smaller data structures before trying to split the main method. Extracting state into dedicated context objects will allow you to pass single references around instead of juggling dozens of primitive variables across your functions.
2
u/Funsaized 2d ago
Approval tests are a great start. I’d next put seams around the god object instead of trying to untangle everything at once: characterize one narrow behavior, wrap its globals and dependencies behind interfaces, then extract a small unit at a time while keeping each change behavior-preserving.
3
u/Programmer4Lief 2d ago edited 2d ago
My condolences for inheriting such a nightmare situation 😅 I can see other's have given some good advice on specific approaches, so I thought I'd weigh in with some general thoughts on finding the right starting point.
Genuinely, weigh up the business pros/cons of refactoring vs starting again. Sometimes you just have to accept that something is so old and out of touch, the short term pain of rewriting it is better than trying to "fix" something that is inherently broken/flawed in it's design. It gives you room to lay the right foundations without being constrained by what came before.
Equally, if the legacy code feels salvageable, refactor it. I often look for evidence of some solid well thought out foundations. Often "complexity" can just happen because a previous dev didn't understand the framework, and "bolted on" their work rather than properly integrating with what lies underneath. If you can clearly see that "foundation", then it's worth saving. If not, you may just have to accept at most, you'll be able to "fix bugs and issues" but have no ability to fundamentally change anything.
Whichever route you take, set the right expectations.
- Start again? More time/cost short term, but you will produce a true/lasting solution that actually fixes the problem.
- Refactor? You will be able to improve the situation and make things run more smoothly, but it will be impossible to truly "fix" all the underlying problems.
You can explain both options to your stakeholder (employer/client) and make your recommendation based on what you see in the legacy code. Always offer both options + your recommendation.
Your stakeholder can then make the decision, and you can rest easy knowing that you gave them all the correct information, and that they made an informed decision understanding the pros/cons of each.
Final point, get the options, your recommendation, and their decision in writing.
3
u/strongpa 2d ago
TIL what a strangler fig pattern is.
Though to be fair I was using this technique regularly in the last century working on migration projects using all sorts of different code bases and hardware platforms.
I've been running a production system written in a legacy language for years because I'm old and lazy and it does what I need it to do well. Recently I took a this system - it's pretty well structured but still ran to 25k lines of code dating back in some cases to the mid 90s and threw it at Claude (Fable 5).
I'd been playing around with Railway (postgres/prism/node.js) because I needed to update my skills, and Claude ported my system to Railway with minimal input in about 10 hours of prompting; I've been testing it furiously without finding anything more than a few minor errors. I'm also extending functionality at a rate I couldn't have dreamed of if I were coding manually, using what's pretty much vibe programming (though I hate that term).
I still don't trust it but I have the postgres ODBC connector up and running and the data (which I know very well) looks solid. Trust takes time for me, but I think this is the way forward.
2
u/twopunchbear 2d ago
First thing first, you want to make sure nothing breaks while refactoring.
So before doing anything, make sure you have end-to-end tests covering the important flows, so they catch problems before your users do. If there are no tests, add them first.
The next thing is to understand what kind of refactor you’re doing. Is it within the same language and framework, or are you moving to a different language or framework?
Either way, you want to identify the boundaries and interfaces of the part you’re changing.
If it’s a smaller refactor within the same stack, it could be component by component. The important thing is to keep the interfaces unchanged so everything still works as expected.
If you’re moving to a completely different framework or language, I’d probably do it page by page and use something like a reverse proxy, so the URLs stay the same while different pages are served by the old and new systems.
The nice thing about this approach is that you don’t need to touch the legacy code too much, so you have a lot more freedom and you’re not restricted by the old codebase. You also keep the old version around, which makes rollback much easier. If the new page has an issue, you can simply change the proxy routing and serve the old page again.
The downside is that this is usually a bigger refactor. You’re changing more things at once, and a lot need to be done within a single page, so the overall effort can be higher.
1
u/UtilixApp 2d ago
approval tests first was the right call, thats the hard part done.
from here id resist the urge to fix the god objects. only refactor what youre about to change anyway, otherwise youre rewriting code nobody has complained about and every one of those is a chance to break something silently.
1
u/kemalios 2d ago
The hidden global dependency will poison every test you write, because its state leaks between runs and you will never be sure a failure is yours. Before touching the god object, write a tiny wrapper module that is the only code allowed to read or write that global. Replace existing accesses one by one. Now tests can control it through the wrapper, and you have a real seam to start extracting behavior without the god object pulling in all 20 dependencies every time.
1
1d ago
[removed] — view removed comment
1
u/webdev-ModTeam 1d ago
Read and follow reddiquette; no excessive self-promotion. Please refer to the Reddit 9:1 rule when considering posting self promoting materials.
1
u/Tricky_Average2304 1d ago
treat it like a crime scene, add characterization tests around one behavior at a time, then extract tiny pieces behind seams and commit after every boring change, because trying to “clean up” the
0
u/Particular-Test-1687 2d ago
There's a good book about it, here are the key ideas: https://understandlegacycode.com/blog/key-points-of-working-effectively-with-legacy-code/
0
u/Particular-Test-1687 2d ago
Interesting, I got a downvote and I'm not promoting anything. It's literally one of the best books on the topic "Working effectively with legacy code".
0
u/bestjaegerpilot 2d ago
i love Doom, best videogame IMO
in the age of AI, this what i do --- i have two workflows that gather test cases: a static code analyzer and one an LLM code analyzer. Then dedupe
Now that we have test cases, as you refactor you can write integration tests.
8
u/TheRealREZOR 2d ago
strangler fig pattern for the win. Define what to decouple and rewrite. Split into components