Sounds like a good example of Chesterton’s Fence, a rule that basically says to never remove a fence until you can explain why it’s there.
The flip side is that refactoring once in a while can clean up undefined behaviors that no one is around anymore to understand or explain. There is also a problem with companies that try to make engineers hot swappable and don’t focus resources on retention, often the people who understand the system’s special complexity are long gone.
That’s no way to maintain a codbase though. Refactoring can’t require investigation that approaches the effort of building the fence in the first place!
That code base will rot out of a fear of touching it.
The answer to the Chesterton rule is: put a note on your fence to explain why that fence is there, and under what conditions it can be removed in the future. (With code you also have the luxury of tests to verify the fence is there so long as the conditions for it being needed still remain)
To be honest, I've never understood Chesterton's Fence - maybe you can help me understand.
On a many codebases I've worked on, it seems prohibitively expensive to research the specific context for every nonsense piece of code before refactoring it. Does it count to say "this code exists because the original author added a lot of nonsense - let's refactor"?
I agree. It's also the case that understanding often comes from changing. So in a sense the fence is just a blanket "never change anything". That's obviously bad.
I think a more reasonable approach is to change stuff in a conservative way where you limit the blast radius. Like running something like Ruby's scientist library on the new code.
> Does it count to say "this code exists because the original author added a lot of nonsense - let's refactor"?
No. Why is the code nonsense? What caused the author to land nonsense vs something different? It's very possible that the authors have very poor taste and/or decision making, or maybe had different understanding of requirements or potential future direction. Understand how they were mistaken and why the refactored version is better.
Unless you're dealing with code which is literal nonsense (i.e. doesn't compile and/or never did what it was supposed to), assume you're mistaken until you see how the code is mistaken instead.
Take a look at the git log to see if you can at least explain one or two reasons why the original author added such non-sense. Was it a feature tackled on at a later date? Did they try to fix some bugs?
If you can't find a reason (or is not willing to do so), it's practically calling the previous programmer either a bad one, or just doesn't care about anything at all. In that case, refactoring might be the least of your concerns.
Checking the vcs history of the file to understand when, and perhaps why, the giant inlined png was added would be a good first step. Usually that will link you to bugs and work items in your issue tracker as well. At the very least you'll get the names of the people responsible, and if all else fails you can go ask them.
Stick around long enough in one place, and you'll end up doing git blame on some gnarly code and discover that the culprit responsible was... yourself.
The flip side is that refactoring once in a while can clean up undefined behaviors that no one is around anymore to understand or explain. There is also a problem with companies that try to make engineers hot swappable and don’t focus resources on retention, often the people who understand the system’s special complexity are long gone.