Mistakes engineers make in large established codebases • Hacker News Board

Re: Mistakes engineers make in large established codebases

#171

Post by jimbokun » Wed, Jan 08, 2025, 5:10 AM UTC

abc-1 wrote: post #136

jimbokun wrote: post #123

Earlier quoted context omitted.

Pulling in lots of dependencies will eventually grind progress on features to a halt as you spend more and more time patching and deploying vulnerabilities. The probability of seeing new vulnerabilities I believe is pretty much linear in the number of dependencies you have.

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.

In house code could very well have many fewer vulnerabilities, as you only write exactly the functionality you need, vs pulling a large dependency and only using a small percentage of the API.

Re: Mistakes engineers make in large established codebases

#172

Post by jrockway » Wed, Jan 08, 2025, 5:18 AM UTC

jimbokun wrote: post #131

jrockway wrote: post #33

Earlier quoted context omitted.

I like keeping things consistent even if the consistent way is "wrong". One thing that bugged me about the large codebase I most recently worked on is that we used a custom assert library for tests. The Go team says this about them: https://go.dev/wiki/TestComments#assert-libraries , and having learned Go at Google, I would never have been allowed to check in code like that. But this place wasn't Google and there wer…

I work on a service where a big percentage of the code is persisting to and reading from various stores, so unit tests have very limited value compared to integration tests.

Yeah, if you can start up the actual things, then you know your code's going to work against the actual things. That's ultimately what we're aiming for.

Re: Mistakes engineers make in large established codebases

#173

Post by bandrami » Wed, Jan 08, 2025, 5:25 AM UTC

peterldowns wrote: post #34

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…

"Now we have five inconsistent coding conventions..."

Re: Mistakes engineers make in large established codebases

#174

Post by Aeolun » Wed, Jan 08, 2025, 5:33 AM UTC

newaccountman2 wrote: post #135

mjr00 wrote: post #17

> 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. You simply cannot redesign any non-trivial project (i.e. a project that makes real money) from first-principles. This resonates. At one former c…

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?

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.

Re: Mistakes engineers make in large established codebases

#175

Post by bandrami » Wed, Jan 08, 2025, 5:36 AM UTC

ge96 wrote: post #19

I'm just thinking about this time at a previous job, I was reviewing a PR and they decided to just find/replace every variable and switch from snake to camel case. I was like "why are you guys doing this, not part of the job". There was some back and forward on that. This is a place where PRs weren't about reviews but just a process to follow, ask someone to approve/not expect feedback. edit: job = ticket task

This. There's consistency and there's yak-shaving. We had a guy who spent two months changing tabs to multiple spaces (or vice versa; whichever was in fashion at the time) mostly to get those sweet commits for "productivity" metrics. Yes, our bad for not realizing that sooner, but at some point you have to let inconsistencies go.

Re: Mistakes engineers make in large established codebases

#176

Post by The_Colonel » Wed, Jan 08, 2025, 5:41 AM UTC

sfink wrote: post #138

peterldowns wrote: post #34

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…

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. If there are 4 ways to do the same thing and you really want to do it a different way, then replace one of those ways with your new one in the process of adding it. For extra credit, get to the po…

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.

Re: Mistakes engineers make in large established codebases

#177

Post by LAC-Tech » Wed, Jan 08, 2025, 5:57 AM UTC

hnthrow90348765 wrote: post #50

To be fair, the previous engineers got paid to write the legacy mess and were employed for a long time if there's a lot of it. Where is the incentive to go the extra mile here? Do you eventually put up with enough legacy mess, pay your dues, then graduate to the clean and modern code bases? Because I don't see a compelling reason you should accept a job or stay in a code base that's a legacy mess and take on this ext…

I'm some what in agreement with this. OP sounds like he has legacy codebase stockholm syndrome. Imagine being appointed mayor of a slum and deciding to build more slums because you wanted to fit in.

Re: Mistakes engineers make in large established codebases

#178

Post by darepublic » Wed, Jan 08, 2025, 6:03 AM UTC

liontwist wrote: post #126

darepublic wrote: post #122

Earlier quoted context omitted.

doing some recent contract work I discovered someone putting this into a PR (comments my own) ``` let susJsonString = '...' // we get this parseable json string from somwhere but of course it might not be parseable. so testing seems warranted... try { // lets bust out a while loop! while(typeof susJsonString === 'string') { susJsonString = JSON.parse(susJsonString) } } catch { susJsonString = {} } // also this was a…

This is totally fine. If you're given shit data this seems like a reasonable way to try to parse it (I would personally bound the loop). Typescript is not going to make it better. The problem is whoever is producing the data.

Why the while loop

Re: Mistakes engineers make in large established codebases

#179

Post by darepublic » Wed, Jan 08, 2025, 6:08 AM UTC

accoil wrote: post #159

bryanrasmussen wrote: post #147

Earlier quoted context omitted.

I think the complaint here is they have a string, which even has the word string in the variable name, and they turn it into an object at the end. Hence references to Typescript. I suppose what is wanted is something like let parsedJSON = {} try { parsedJSON = JSON.parse(susJsonString) } catch { //maybe register problem with parsing. }

That's quite different though. It looks to be dealing with the case that a serialised object gets serialiased multiple times before it reaches that point of code, so it needs to keep deserialising until it gets a real object. E.g: JSON.parse(JSON.parse("\"{foo: 1}\"")) I'd guess the problem is something upstream.

The code is either going to loop once and exit or loop forever no

Re: Mistakes engineers make in large established codebases

#180

Post by zwnow » Wed, Jan 08, 2025, 6:14 AM UTC

sfink wrote: post #138

peterldowns wrote: post #34

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…

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. If there are 4 ways to do the same thing and you really want to do it a different way, then replace one of those ways with your new one in the process of adding it. For extra credit, get to the po…

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.