Mistakes engineers make in large established codebases

Thats not how software engineering works in a business setting though? Not a single company I have been in has the time to first fix the existing codebase before adding a new feature. The new feature is verbally guaranteed to the customers by project managers and then its on the dev to deliver within the deadline or you'll have much greater issues than a inconsistent codebase. I'd love to work in a fantasy company that allows for fixing legacy code, but that can take months to years with multi million line codebases.

I don't disagree with what you've written at all, but let me just say:

> Hopefully, you have a monorepo or something with similar effects, and a lack of fiefdoms

ah to be so lucky...

Great points, I'd just add:

> A lot of inconsistency is the result of unwillingness to fix other people's stuff

Agree, so we find it best to practice "no code ownership" or better yet "shared code ownership." So we try to think of it all as "our stuff" rather than "other people's stuff." Maybe you just joined the project, and are working around code that hasn't been touched in 5 years, but we're all responsible for improving the code and making it better as we go.

That requires a high trust environment; I don't know if it could work for Firefox where you may have some very part-time contributors. But having documented standards, plus clang-format and clang-tidy to automate some of the simpler things, also goes a long way.

I agree, but this presupposes a large comprehensive test suite giving you enough confidence to do such sweeping changes. I don't doubt Firefox has it, but most (even large, established projects) will not. A common case I've seen is that newer parts are relatively well covered, but older, less often touched parts don't have good coverage, which makes it risky to do such sweeping changes.

> In that case, if the current way is undocumented and/or inconsistent, you make it better before or while adding in your new approach.

Sometimes, but oftentimes that would involve touching code that you don't need to touch in order to get the current ticket done, which in turn involves more QA effort.

> Hopefully, you have a monorepo or something with similar effects, and a lack of fiefdoms. In that case, if the current way is undocumented and/or inconsistent, you make it better before or while adding in your new approach.

Unfortunately, this is how you often get even more inconsistent codebases that include multiple tenures' worth of different developers attempting to make it better and not finishing before they move on from the organization.

It depends on the day but generally I believe that most engineers want to write good code, want to improve their own skills, and like learning and critiquing with other engineers. Sometimes a small catalyst is all it takes to dramatically improve things. Most of the times I've thought that individual contributors were the problem, the real issue was what the company's leaders were punishing/rewarding/demanding.

>Not really my experience in teams that create inconsistent, undocumented codebases... but you might get 1 or 2 converts.

This has also been my experience. Usually there is a "Top" sticky/unhelpful/reticent person. They are not really a director or exec but they often act like it and seem immune from any repercussions from the actual higher ups. This person tends to attract "followers" that know they will keep their jobs if they follow the sticky person for job security. There usually are a few up and coming people that want better that will kinda go along with you for their own skill building benefit but its all very shaky and you can't count on them supporting you if resistance happens.

I've literally had the "I was here before you and will be after" speech from one of the "sticky's" before.

All these HN how to do better write ups seem to universally ignore the issues of power and politics dynamics and give "in a vacuum" advice. Recognizing a rock and a hard place and saving your sanity by not caring is a perfectly rational decision.

ahh, there's a lot of scenarios here.

in my scenario, those people were gone.

I’ll check out that book, thanks for the reference.

And it's fine, right? Honestly I think people need to realize that part of being a good engineer is being able to deal with inconsistency. Maybe submodule A and submodule B do network requests slightly differently but if both ways are reasonable, working, and making the company money, it's probably not worth delaying product improvements in order to make things "more consistent."

On the other hand if no one in your company cares about consistency, at some point everything becomes so awful you basically won't be able to retain engineers or hire new ones, so this is a place where careful judgement is needed.

And to be practical, that's fine. In a big codebase it's more important to encourage consistent, well-defined, small interfaces, and a clean separation of concerns, than to try to get consistency in the lower-level implementation details. Other non-code concerns like coordinating releases and migration of shared services are also way more important than getting everyone to use the same string library.

(Of course, if you carry that principle to the extreme you end up with a lot of black-box networked microservices.)

If there's resistance to picking a style guide, autoformatting might be a viable start and will probably do quite a bit for shallow consistency at the price of large PR:s once per file. Once one has worked with a forced style for a while it starts to feel weird to see breaches of it, and I think that might help softening people to adapting a style guide regarding more subtle things like error handling or attitude to standardised protocols like HTTP.

Highly agree. I've done quite a few large refactors of unnecessarily complex systems that resulted in significant improvement - from lots of bugs to nearly no bugs, incomprehensible code to simple straight forward code, no tests to great test coverage.

I did have one bad experience where I ended up spending way too much time on a project like that, I think I made some mistakes with that one and got in a bit too deep. Luckily my team was very supportive and I was able to finish it and it's a lot better now than it was.

Counter-thought:

We are making brand new things here and not being in an assembly line coming up with the very same thing dozens to million times. We are paid to make new products never existed, having novelty elements in it desired to be a bigger extent than not!

Those pretending knowing exactly what they are doing are lying!

Of course we are speculating here about the size of novelty content to a differing extent, which is never 0% and never 100%, but something inbetween. But those pushing back on those at least trying to revise the work - putting emphasis on it -, deserve no-one coming to them to be pushed back (at least for the inability of allocating resources for this essential activity of development. Development!).

(tried to mimic the atmosphere of the message, sorry if failed)

You know so little about the team, the organisation, the codebase, other concurrent obligations (e.g. prod support), and the way the project is run. The only way I can imagine one having confidence in a statement like "nothing should take that long" is naïveté.

It really depends. Honestly 6-9 months would have been an optimistic estimate even if it were 2-4 devs intimately familiar with the existing codebase. Permissions is a very cross-cutting concern, as you might imagine, and touched a huge amount of the monolith code. A big problem was that permissions checks weren't done in a consistent layer, instead scattered all over the place, and the team responsible for the rewrite, being new to the code, was starting from scratch and finding these landmines as they went. Scoping was also unclear and kept changing as the project went along, at first to pull in more scope that was peripherally related, then to push stuff out of scope as the project went off track. And during all these changes you have to keep the existing auth system working with zero downtime.

The devs were good developers, too! Two people on the team went off to Google after, so it's not like this was due to total incompetence or anything; more just overambition and lack of familiarity with working on legacy code.

Maybe. There's a lot of dragons hidden inside enterprise code. Only if you know all of them can you really succeed the first time around.

At a large enterprise, 6-9 months is blazingly fast.

Everything takes longer than you think and this sounds like it involves at least 2 teams (the php team and the scalar team). Every team you include increases time line factorially in the best case.

It takes a lot of time to have meetings with a dozen managers to argue over priority and whatever. Especially since their schedules are full of other arguments already

authorization and access control is an awfully difficult problem, as soon as you derive from user defined ACLs on all persisted objects and user/groups based granting data. Each access can have an arbitrary rule that must be evaluated with all dependant data, that will end up being anything in the persisted data. How to you make this long rule list maintainable, without redundancy, ensuring that changing re-used rules won't introduce regressions on all call sites ?

> Am I naive for thinking that nothing like that should take as long as 6-9 months in the happy case and that it's absurd for it to not succeed at all?

Bluntly, yes. And so is every other reply to you that says "no this isn't naive", or "there's no reason this project shouldn't have finished". All that means is that you've not seen a truly "enterprise" codebase that may be bringing in tons of business value, but whose internals are a true human centipede of bad practices and organic tendrils of doing things the wrong way.

Authz can make the most otherwise trivial features into a depressing journey in the valley of edge cases.

No, you’re not naive. If it’s done by one-two people that know what they’re doing, it should be done much faster.

If it’s a big new team that doesn’t know what they’re doing, working separately from existing codebase, with lots of meetings… I see no reason why it would finish at all.

> Meanwhile the developer that was quietly doing ongoing maintenance on the monolith had gradually broken out some key performance-critical elements into Scala and migrated away from the Ruby code for those features.

Yep and that's what I've seen be successful: someone who really knows the existing code inside and out, warts and all, needs to be a key leader for the part being broken out into a separate system. The hard part isn't building the new system in these projects, it's the integration into the existing system, which always requires a ton of refactoring work.

Even if you’re generally suspicious of so called best practice/ design patterns / Martin Fowlerisms.. this is a time for the strangler approach. (Parent and siblings are already talking about the same idea without naming it.)

Rewrites from scratch never work with sufficiently large systems, and anyone that’s been involved with these things should be savvy enough to recognize this. The only question is around the exact definition of sufficiently large for a given context.

https://en.m.wikipedia.org/wiki/Strangler_fig_pattern

Also an issue is that the director attempted a full rewrite as a separate project.

You can do successful rewrites but your rewrite has to be usable in production within like a month.

If you don’t know how to achieve that, don’t even try.

The quiet developer was able to get their own rewrite done because they understood that.

Looks like the director of engineering showed some classic inexperience. You can tell when someone has done something before and when it’s their first time.

This is the way. You absolutely can turn shit codebases into something nicer and faster, and this is best done by people who know the system, and maybe even more important, knows the operational context the system exists in.

I once came into an old codebase like this as a junior, thinking I can start again. And I was gently but firmly told by my boss that it wouldn't work, this software is crucial to operations that support all our revenue, and while it can be improved it has to keep working. And we improved the hell out of it.

I always took it as "leave it better than you found it" across the files that I've been working on (with some freedom as long I'm on schedule). My focus is to address the ticket I'm working on. Larger improvements and refactorings get ticketed separately (and yes, we do allocate time for them). In other words, I don't think it's misguided.

I do not believe in "boyscouting". I think if you want to leave it better, make a ticket and do it later. Tacking it on to your already planned work is outside the scope of your original intent. This will impact your team's ability to understand and review your changes. Your codebase is unlikely to be optimized for your whimsy. Worse though is when a reviewer suggests boyscouting.

I've seen too many needless errors after someone happened to "fix a tiny little thing" and then fail to deliver their original task and further distract others trying to resolve the mistake. I believe clear intention and communication are paramount. If I want to make something better, I prefer to file a ticket and do it with intention.

This. So much this.

> Being able to navigate and reverse engineer undocumented legacy code in a non-modern stack is a skill set in and of itself.

And I find that it's a pretty rare skill to find.

> People often ask why I hardly ever have any prod issues (zero so far this year).

It also helps that we're still only in January!

> zero so far this year

I saw what you did there.

> Some codebases make me feel like I’m reading a book in multiple languages …

In most cases the codebase does consist of muliple languges.

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.

But that’s a judgment call based on experience.

>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.

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.

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.

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.

> Any OS is probably more than 5M lines (Linux - 27.8 lines according to a random Google Search).

Linux is notoriously fragmented/duplicative, and an OS isn't the solution to anyone's actual business problem. A well-factored solution to a specific problem would be much smaller, compare e.g. QNX.

> Facebook is probably more lines of code.

IIRC Facebook is the last non-monorepo holdout among the giants, they genuinely split up their codebase and have parts that operate independently.

Does Facebook have more than 5M lines of code now? I'm sure they do. Does that actually result in a better product than when it was less than 5M lines of code? Ehhhh. Certainly if we're talking about where the revenue is being generated, as the article wants to, then I suspect at least 80% of it is generated by the <5M that were written first.

So I mean yeah, on some level solving the business problem can take as many lines as you want it to, because it's always possible to add some special case enhancement for some edge case that takes more lines. But if you just keep growing the codebase until it's unprofitable then that's not actually particularly valuable code and it's not very nice to work on either.

As opposed to your in-house code which is vulnerability free?

The issue isn’t vulnerability's, it’s dependency hell where all your packages are constantly fighting each other for specific versions. Although some languages handle this better than others.

I don't think the examples add up. Like, yes, if the system has a way to do auth then you should use it. But if the system's way of doing auth has nasty surprises, rather than cargo-cult the workaround to those nasty surprises, you should fix them! (Especially if the article wants to argue that adding inconsistency is bad - then by the same token removing the inconsistency that someone added before you is good). And if the system has its own custom auth implementation that does the same thing as a standard library, you should probably pull it out completely and replace it with that standard library.

I've successfully pulled off most of such a majority re-write but a key driver - but not the only was that the legacy language the existing system was implemented in had lost virtually all traction in the local and global market. Mostly only expensive contractors coming out of pension availabile and on top of that the custom libraries required us to recruit the 10 percent of that segment. Any new hires straight up refused to pick it up as they accurately deemed it career suicide.

> Build stuff that does exactly what you want it to do, nothing more

This is also confusing to me. In a multi-million line codebase, it's extremely difficult to find an actual place where you have zero side effects with ANYTHING you write.