Live data from Hacker News

Mistakes engineers make in large established codebases

seangoedecke.com

81–90 of 384 posts

Re: Mistakes engineers make in large established codebases

#81
I once worked on a large project in the past where it took 3 days to rename a field in an HTTP response because of how many services and tests were affected. Just getting that through QA was a huge challenge.

Working in a large dev team, focusing on a small feature and having a separate product manager and QA team makes it easier to handle the scale though. Development is very slow but predictable. In my case, the company had low expectations and management knew it would take several months to implement a simple form inside a modal with a couple of tabs and a submit button. They hired contractors (myself included), paying top dollar to do this; for them, the ability to move at a snail's pace was worth it if it provided a strong guarantee that the project would eventually get done. I guess companies above a certain size have a certain expectation of project failure or cancellation so they're not too fussed about timelines or costs.

It's shocking coming from a startup environment where the failure tolerance is 0 and there is huge pressure to deliver on time.

Re: Mistakes engineers make in large established codebases

#82

I never really understood putting consistency on a pedestal. It's certainly nice when everything operates exactly the same way - but consistency for consistency's sake is awful to work in too. If a team realizes that logging library B is better than library A, and but NEVER switches from A to B because of consistency concerns, then in two years they'll still all be using inferior tools and writing worse code. Similar…

"Consistency for consistency's sake" is usually a misinterpretation of "Consistency because there are reasons for the way things are already done and you don't understand those reasons well enough to diverge". If you understand the current system completely, then you can understand when to diverge from the system (though this is usually better expressed as "when to improve the system" rather than doing something completely new). If you don't understand the current system, then you can't possibly ensure that you haven't missed something in your shiny new way of doing things.

Re: Mistakes engineers make in large established codebases

#83

I agree that consistency is important — but what about when the existing codebase is already inconsistent? Even worse, what if the existing codebase is both inconsistent and the "right way to do things" is undocumented? That's much closer to what I've experienced when joining companies with lots of existing code. In this scenario, I've found that the only productive way forward is to do the best job you can, in your…

My approach is what I call defensive programming, with a different meaning than the usual usage of the term. I assume that my coworkers are idiots that aren't going to read my documentation, so I make all public classes and methods etc. as idiot-proof as possible to use. Hasn't saved me from every issue caused by my teammates never reading my docs or asking me questions, but it's definitely prevented several.

Re: Mistakes engineers make in large established codebases

#84
>The other reason is that you cannot split up a large established codebase without first understanding it. I have seen large codebases successfully split up, but I have never seen that done by a team that wasn’t already fluent at shipping features inside the large codebase

I cannot resonate with this. Having worked with multiple large code bases 5M+, splitting the codebase is usually a reflection of org structure and bifurcation of domain within eng orgs. While it may seem convoluted at first, its certainly doable and gets easier as you progress along. Also, code migrations of this magnitude is usually carried out by core platform oriented teams, that rarely ship customer-facing features.

Re: Mistakes engineers make in large established codebases

#85
post #48

Earlier quoted context omitted.

What was the established code style (...if any) in that project? Anyway it doesn't sound like that was a very mature project or developers, not when the reviewer decide to just edit code instead of provide a review.

the old/existing code was all underscore, they wanted to use camelcase instead. it's a dumb thing to be argue about I know but it made the code review harder when instead of 10s of line diffs there's almost a hundred granted easy to see just changing casing

I just insist that style only changes go in a separate commit.

Re: Mistakes engineers make in large established codebases

#86
Consistency is often helpful, but you also need to be wary of cargo culting. For example, you see a server back end that uses an ORM model and you figure you'll implement your new feature using the same patterns you see there. Then a month later the author of the original code you cribbed comes by and asks you, "just out of curiosity, why did you feel the need to create five new database tables for your feature?"

I know, that's a pretty specific "hypothetical," but that experience taught me that copying for the sake of consistency only works if you actually understand what it is you're copying. And I was also lucky that the senior engineer was nice about it.

Re: Mistakes engineers make in large established codebases

#87
post #6

Earlier quoted context omitted.

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 impl…

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

Bahh thanks for the chuckle. The man is 7/7 as of today!

Re: Mistakes engineers make in large established codebases

#88

I agree that consistency is important — but what about when the existing codebase is already inconsistent? Even worse, what if the existing codebase is both inconsistent and the "right way to do things" is undocumented? That's much closer to what I've experienced when joining companies with lots of existing code. In this scenario, I've found that the only productive way forward is to do the best job you can, in your…

> In this scenario, I've found that the only productive way forward is to do the best job you can, in your own isolated code, and share loudly and frequently why you're doing things your new different way.

Now you have N+1 ways.

It can work if you manage to get a majority of a team to support your efforts, create good interfaces into the legacy code paths, and most importantly: write meaningful and useful integration tests against that interface.

Michael Feathers wrote a wonderful book about this called, Working Effectively with Legacy Code.

I think what the author is trying to say with consistency is to avoid adding even more paths, layers, and indirection in an already untested and difficult code base.

Work strategically, methodically, and communicate well as you say and it can be a real source of progress with an existing system.

Re: Mistakes engineers make in large established codebases

#89
post #58

Earlier quoted context omitted.

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.

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.

Re: Mistakes engineers make in large established codebases

#90

I once worked on a large project in the past where it took 3 days to rename a field in an HTTP response because of how many services and tests were affected. Just getting that through QA was a huge challenge. Working in a large dev team, focusing on a small feature and having a separate product manager and QA team makes it easier to handle the scale though. Development is very slow but predictable. In my case, the co…

Only 3 days? That's incredible.

Getting a PR reviewed in 3 days is an achievement!

Post reply on HN