I have this bad codebase at work. Really bad. One of the things I’ve been working on for the past two years is making it consistent. I’m almost at the point where interfaces can be left alone and internals rewrites in a consistent style.
People often ask why I hardly ever have any prod issues (zero so far this year). This is part of the reason. Having consistent codebases that are written in a specific style and implement things in similar manner.
Some codebases make me feel like I’m reading a book in multiple languages …
I don't like this philosophy as it often leads to stagnation in patterns and ways of working that seep into newer systems. "That's not how we do things here" becomes a common criticism, resulting in systems and services that share the same flaws and trade-offs, making progress difficult.
Engineers often adhere too rigidly to these principles rather than taking a pragmatic approach that balances existing practices with future improvements.
Therein lies the rub. Everyone has a different idea of what is an improvement in a codebase. Unless there's some performance or security concern, I'd much rather work in an "old" style codebase that's consistent than a continually partially updated codebase by multiple engineers with different opinions on what an "improvement" is.
> I don't like this philosophy as it often leads to stagnation in patterns and ways of working that seep into newer systems.
The rule isn't "don't introduce change", it's "be consistent". Using the example from the post, if you want to use a different method of doing auth that simpler the "be consistent" rule means you must change the way auth is done everywhere.
Interestingly, if you do that the negatives he lists go away. For example, if the global auth mechanism handles bots specially, you will learn that if you are forced to change it everywhere.
What's bad is code exhibiting multiple fragmentary inconsistencies,
and no plan or effort exists to bring older code up to match the new patterns.
An example I was closely involved with:
A java programmer who wrote a plethora of new code in a pure functional paradigm,
scattering Vavr library uses all over it.
The existing code was built on Dropwizard
and any experienced Java programmer could rapidly get comfortable with it.
The difference between the existing code and the new was jarring (sorry for the pun) to say the least,
and I wonder if, later, the company ever managed to recruit anyone who understood both well enough to maintain the system.
ETA: upon reflection I'd consider that programmer a canonical example of the kinds of mistakes the author covers in the article.
And that's a fair criticism, however, if you change a pattern without changing it everywhere, you now have two patterns to maintain (the article mentions this). And if multiple people come up with multiple patterns, that maintenance debt multiplies.
Progress and improvement is fine, great even, but consistency is more important. If you change a pattern, change it everywhere.
Change it at once everywhere on an existing large codebase? That's going to be one huge PR no one will want to review properly, let alone approve.
Document the old pattern, document the new pattern, discuss, and come up with a piece by piece plan that is easy to ship and easy to revert if you do screw things up.
Unless the old pattern is insecure or burns your servers, that is.
I don’t think you and the comment you are replying to are in conflict. Documenting and rolling it out piecemeal is the correct way to make a large change.
I think the point is, either actually commit to doing that, or don’t introduce the new pattern.
Yes, it usually can't be done with one massive checkin. First get buy-in that the old pattern will be changed to the new pattern (hopefully you won't have some senior developer who likes it the old way and fights you), then as you say, come up with a plan to get it done.
The down side to this that I've experienced more than once, though, is incomplete conversions: we thought we had agreement that the change should be done, it turns out to be more difficult than planned, it gets partially completed and then management has a new fire for us to fight, resources are taken away, so you still have two or more ways of doing things.
If things are consistent enough, tools like open rewrite can be used. I’ve seen reviews where it is the recipe for generating the code transformation that gets reviewed, not the thousands of spots where the transform was applied.
Yep when I have to work on old code I find something in the existing code that's close to what I want to do, and copy/paste it. I do not try to abstract it into a common function, unless that's already been done and can be used verbatim.
You don't know the 10 years of reasons behind why the code is the way it is, and the safest thing is to stay as close as possible to how the existing code is written, both to avoid landmines, and so that future you (or someone else) has one less peculiar style they have to figure out.
All that said, the more usual case is that the code is already a huge mess of different styles, because the 100 different developers who have touched it before you didn't follow this advice.
Do you ever find yourself taking an existing function and adding another parameter to it? The benefit is that you don’t break existing code. The problem is that the function is now more complicated and likely now does more than one thing because the extra parameter is effectively a mode switch.
The only time I abstract something into a function even if it’s only used in one place, is if the generic case is as easier or easier than the code for the specific case.
How do you tackle the case where the codebase is consistent in a bad way, like pervasive use of antipatterns that make code difficult to change or to reason about? If you want to improve that, you have to start somewhere. Of course, Chesterton’s Fence applies.
Or when people keep the old pattern because there's a "higher priority".
I've worked on a project with absolutely terrible duplication of deserialisers of models, each one slightly different even those most properties were named the same and should've been handled the same. But we can't touch anything because important things are happening in business and we can't risk anything. The ignored part was that this led to bugs and extreme confusion from new people. They were even too worried to accept a repo-wide whitespace normalisation.
In the past I have performed needed refactorings as part of new feature development, without asking permissions. Even though my boss at the time said “don’t make it pretty, just make it work.”
Of course, I knew that writing the code the “pretty”, more maintainable, easier to understand way wouldn’t take any longer to write, and might take less time as the refactoring requires less new code overall.
But I didn’t bother explaining all that. Just nodded then went and implemented it the best way as I was the experienced software engineer.
