You will not get a greenfield. You will get a repo that grew for six years, with a folder named new-new-api, a helper called utils2, and a comment that says DO NOT TOUCH above the billing module. The professional move is not a rewrite. It is to leave each change a little clearer than you found it, without boiling the ocean in a side quest.
This is the Boy Scout rule with adult constraints: scope, risk, and respect for the people who ship next week.
Do not rewrite it in your first month
You do not understand the business rules yet. The ugly code might encode a tax exception for a state you have never heard of. A rewrite will drop it. Then you will learn about the state from a lawyer.
Read the tests, the tickets, the git blame for the scary module. Add logs if you must. Then change what you came to change.
A rewrite can be right after you have measured: this module is the incident factory, the original authors are gone, and the tests exist or you will write them first. Even then, strangle it: new path beside the old, switch traffic, delete the old. Big bang rewrites in the same process are how companies stall.
The scout move on a real PR
You are there to fix a bug in calculateTotal. You find a variable named x. You rename it in that function. You add a test for the bug. You do not reformat the 2,000-line file. You do not rename the class. You do not migrate the file to a new folder.
If the file is so bad you cannot fix the bug safely, then the cleanup is in scope. Say so in the PR: “I had to split this function to test the discount branch.” That is honest. “While I was here, TypeScript and Prettier and a new folder” is how reviewers stop trusting you.
Names, deletes, and dead code
Delete code that is unreachable if you can prove it. git grep the symbol. If it is only called from itself and a test that imports it, it might still be a public API. Check exports.
Rename when the name lies. getUser that writes to the database should not stay getUser. The rename is the documentation.
Comments that explain why a weird thing exists are gold. Comments that restate the code are noise. Replace the noise with a better name.
Boundaries, not micro-packages
Messy code is often missing a boundary: HTTP, domain, database all in one file. When you touch it, extract the smallest function that has a name in the domain: applyCoupon, isEligibleForFreeShipping. Leave it in the same file if a new file would be theater. A 40-line well-named function in a messy file is still a win.
Do not introduce a 12-layer architecture for a script. Match the neighborhood. A pristine hexagonal module in a sea of scripts is a trap for the next editor.
Tests as permission to clean
If there are no tests, write one characterization test: assert current outputs for a few known inputs, even if the outputs look wrong. Then refactor. Then fix the bug. If you “fix” and refactor at once, you will not know which step broke the tax rule.
Golden files for messy HTML can work. They are brittle. Prefer asserting the number and the status code.
Formatting and the holy war
One formatting PR for the whole repo, agreed, with no logic. Then never again. Mixing format and logic is how you hide a one-line security fix.
If the project has no formatter, adding one is a gift if the team agrees. If they do not, follow the file.
Documentation that is next to the danger
A README in the billing folder with “watch tax, see ticket 441” is better than a Confluence novel. Update it when you learn. Delete it when it lies.
Architecture Decision Records for the big choices: why we kept the monolith, why we use this queue. Not for every function.
Social rules
Ask who owns the module. Tell them you are cleaning a function, not the product. Credit the ugly workaround in the commit if it saved a customer: “kept the 2019 leap-year hack, see comment.” Respect is part of leaving it better. Contempt in comments is not a refactor.
If you find a security issue, do not “quietly fix” in a drive-by if it needs a CVE process. Follow the team’s disclosure. Still fix it.
A definition of better
- A name that matches behavior
- A test for the thing you changed
- A deleted unused branch
- A comment that explains a constraint
- A slightly smaller function
Not: a new framework, a new folder tree, a rewrite of styles.
Feature flags as a cleanup tool
If you must add a new path through a mess, put it behind a flag, ship it to staff, then delete the old path when the flag is 100%. Deleting is the cleanup. Flags that live forever are a second mess. Calendar the removal.
Do not use a flag to hide an incomplete migration from yourself for six months without a ticket. That is how you get two sources of truth.
Measuring “better”
If you cleaned a hot function, you can look at error rates and time-to-change the next ticket in that file. If the next person spends less time finding the discount rule, you won. You will not get a medal. You will get a quieter blame.
Dependencies you should not “clean”
Do not upgrade a major framework in the same PR as a bugfix. Do not change the linter rules for the whole repo to make your file pass. Do not reformat YAML from a Helm chart you do not own.
If a library is unmaintained and you must vendor a patch, isolate it and comment the upstream issue. That is leaving it better. A silent fork in utils is leaving it worse.
When cleanup is the ticket
Sometimes the ticket is the mess. Then you can take a larger bite: extract a module, add tests, delete the dead flag. Still ship in slices. A week of cleanup with no merge is a rewrite in disguise. Merge something every day or two so others can work.
Comments that aged into lies
When you change behavior, change the comment in the same commit. A wrong comment is worse than none. If you cannot explain the why in a comment, you might not understand the change yet. That is a signal to slow down, not to delete the comment and hope.
Leave the ticket better too
If the ticket was vague, add the reproduction you found. The next person in the messy module will not have to rediscover it. That is part of leaving the campground better. Code is not the only artifact.
Artikals is for people living in inherited code, which is most people. Legacy is another word for “made money.” Treat it like that. Leave a trail of small honest improvements. In a year the file is still messy and also safer. That is a career. The greenfield can wait until you have earned the right to throw something away.
