Hacker News Reader

Hacker News

Comment by jonahx | Hacker News Reader

Parent Full thread

jonahx

Preface: I was also very glad to see this called out specifically, and think it's a great rule.

That said...

> Note how Google does NOT say "make sure the code is properly architected".

is not accurate. The very first paragraph on the same page is:

> Design

> The most important thing to cover in a review is the overall design of the CL. Do the interactions of various pieces of code in the CL make sense? Does this change belong in your codebase, or in a library? Does it integrate well with the rest of your system? Is now a good time to add this functionality?

Emphasis mine. But that sure sounds like they care about proper architecture. They just also care about avoiding over-engineering. They're not mutually exclusive.

View on HN

But that sure sounds like they care about proper architecture.

Of course they care about architecture—I don’t think anyone implied otherwise. But at good companies like Google good architecture is a given. Top developers often fall into the pit of over-engineering, and almost never under-architect. So at top companies code reviewers have to be vigilant about over-engineering and rarely have to worry about under-architecting.

So for anyone, including me, when looking out for the next job should be:

- Is `good architecture` a given in this team?

- What are they doing to avoid `over engineering`?

What are the specific questions one can ask to find out the answers to the above two?

>But at good companies like Google good architecture is a given.

Absolutely not. Google’s interview process and inflow of fresh graduates does not bode well for good architecture. Having spent time at G and FB, I can certainly tell you that employees at both are no better at architecting code in a sane way than SWEs at other companies.

Code architecture requires experience. Google does not.

Most teams at Google have senior devs that ensure this. The average level is way higher than other companies.

I think the hiring bar at places like Google and FB is astronomically higher than almost all other places - they definitely have generally higher skilled people there on average.

The process does not encourage experience but mainly the ability to remember recent academic programming puzzles. The first comment is correct even though they do have many existing experienced developers that would be in a position to provide architectural advice.

Checking for memorized algorithms and an MIT degree do not guarantee good code.

I've seen more horrible code at a FAANG company than in many other places. Complete absence of overflow analysis in C++, race conditions, architecture astronauts, exploding code bases that no one understands any more.

Features are being added on a daily basis, basically what matters is a high line count.

You clearly haven't seen code at Google and its "base" library.

The hiring bar at Google does not test for the ability to organize code coherently. The skill is completely unrelated to algorithms and data structures.

It’s like requiring candidates to deadlift 500 lbs and then assuming it means they can all run marathons.

what does the _CL_ notation indicate?

Changelist, perhaps?

Thanks. This would be a nice thing to explain in the document that's open sourced.

The HN submission links directly to https://google.github.io/eng-practices/review/reviewer/ but if you start at the top namely https://google.github.io/eng-practices/ then there's a Terminology section right there on the first page. Maybe the GitHub Pages theme used by this site should be changed to one in which where every page links to its parent page (right now there doesn't seem to be any way to navigate upwards except by editing the URL).

"changelist" is the term perforce uses for a a single commit before it gets committed. A bit like a PR in git. Google used perforce before rolling their own vcs, and they kept the perforce terminology.

Reply on news.ycombinator.com