Live data from Hacker News

Mistakes engineers make in large established codebases

seangoedecke.com

31–40 of 384 posts

Re: Mistakes engineers make in large established codebases

#31
post #3

The "The cardinal mistake is inconsistency" is 100% true. We used to call the guiding philosophy of working in these codebases "When in Rome".

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.

Re: Mistakes engineers make in large established codebases

#32
post #10

I really liked this: "as a general rule, large established codebases produce 90% of the value." People see the ugliness -- because solving real problems, especially if business practices are involved, is often very messy -- but that's where the value is.

I also find amusing that “legacy” more often than not gets used in negative conotation. I hear “legacy” and I think “bunch of people wrote some AWESOME shit that lasted so long that now other people get to view it as ‘legacy’”

There's a good chance that's not what people mean by this term though.

It's probably used in the (now) classic sense as defined by M. Feathers in his "Working with legacy code" book.

Code that is old but otherwise awesome, maintainable (or even actively maintained) and easy / a joy to work with are rarely referred to as "legacy code".

Re: Mistakes engineers make in large established codebases

#33
post #23

There was only one mistake that the article felt like giving a header to: "The cardinal mistake is inconsistency" The instinct to keep doing things the wrong way because they were done the wrong way previously is strong enough across the industry without this article. I love to > take advantage of future improvements. However, newer and better ways of doing things are almost invariably inconsistent with the establish…

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 were tens of thousands of lines of these tests, so I told new developers to keep doing things the "wrong" way. This didn't cause many problems, even if failing tests failing too soon is pretty annoying. Most of the time the tests pass, and the yes/no signal is valuable even if you can debug more by simply `t.Errorf(...)` and continuing.

As for starting databases during tests, it's saved me a lot of trouble over the years. One time, we used sqlite for tests and Postgres for production. We had some code that inserted like `insert into foo (some_bool) values ('t')` and did a query like `select * from foo where some_bool='true'`. This query never matched rows in the tests, because t != true in SQLite, but t == true in Postgres. After that, I found it easier to just run the real database that's going to be used in production for tests. The only thing that behaves identically to production is the exact code you're running in production.

Over here, I have code that uses a hermetic Postgres binary (and chain of shared libraries because Postgres hates static linking) that starts up a fresh Postgres instance for each test. It takes on the order of a millisecond to start up: https://github.com/jrockway/monorepo/blob/main/internal/test.... The biggest problem I've had with using the "real" database in tests is low throughput because of fsync (which `perf` showed me when I finally looked into it). Fortunately, you can just disable fsync, and boy is it fast even with 64 tests running in parallel.

One thing that's been slow in the past is applying 50 migrations to an empty database before every test. When you have one migration, it's fast, but it's one of those things that starts to slow down as your app gets big. My solution is to have a `go generate` type thing that applies the migrations to an empty database and pg_dumps resulting database to a file that you check in (and a test to make sure you remembered to do this). This has two benefits; one, tests just apply a single SQL file to create the test database, and two, you get a diff over the entire schema of your database for the code reviewer to look at during code reviews. I've found it incredibly useful (but don't do it for my personal projects because I've been lazy and it's not slow yet).

Overall, my take on testing is that I like an integration test more than a unit test. I'd prefer people spend time on exercising a realistic small part of the codebase than to spend time on mocks and true isolation. This is where a lot of bugs lie.

Of course, if you are writing some "smart" code and not just "glue" code, you're going to be writing a lot of unit tests. Neither replaces the other, but if you can spend 30 seconds writing a test that does actual database queries or 2 weeks mocking out the database so the test can be a unit test instead of an integration test, I'd tell you to just write the integration test. Then you know the real code works.

Re: Mistakes engineers make in large established codebases

#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 own isolated code, and share loudly and frequently why you're doing things your new different way. Write your code to be re-used and shared. Write docs for it. Explain why it's the correct approach. Ask for feedback from the wider engineering org (although don't block on it if they're not directly involved with your work.) You'll quickly find out if other engineers agree that your approach is better. If it's actually better, others will start following your lead. If it's not, you'll be able to adjust.

Of course, when working in the existing code, try to be as locally consistent as possible with the surrounding code, even if it's terrible. I like to think of this as "getting in and out" as quickly as possible.

If you encounter particularly sticky/unhelpful/reticent team members, it can help to remind them that (a) the existing code is worse than what you're writing, (b) there is no documented pattern that you're breaking, (c) your work is an experiment and you will later revise it. Often asking them to simply document the convention that you are supposedly breaking is enough to get them to go away, since they won't bother to spend the effort.

Re: Mistakes engineers make in large established codebases

#35
post #31
post #3

The "The cardinal mistake is inconsistency" is 100% true. We used to call the guiding philosophy of working in these codebases "When in Rome".

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

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.

Re: Mistakes engineers make in large established codebases

#36
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…

I worked at a company that had a Rails monolith that underwent similar scenario. A new director of engineering brought in a half dozen or of his friends from his previous employer to write Scala. They formed up a clique and decide Things Were Going to Change. Some 18 months and 3 projects later, nothing they worked on was in production. 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. Not only had it gone into production, it made maintenance far easier.

Re: Mistakes engineers make in large established codebases

#37
post #10

I really liked this: "as a general rule, large established codebases produce 90% of the value." People see the ugliness -- because solving real problems, especially if business practices are involved, is often very messy -- but that's where the value is.

Earning code trumps pretty code every time.

Re: Mistakes engineers make in large established codebases

#38
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

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.

Re: Mistakes engineers make in large established codebases

#39
post #31
post #3

The "The cardinal mistake is inconsistency" is 100% true. We used to call the guiding philosophy of working in these codebases "When in Rome".

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

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

Re: Mistakes engineers make in large established codebases

#40

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…

[deleted]
Post reply on HN