4 ms·

It can be tricky to find a balance point here. At Google scale (in time-of-maintenance, not space... Code can last for years and will be worked on by multiple e

It can be tricky to find a balance point here. At Google scale (in time-of-maintenance, not space... Code can last for years and will be worked on by multiple engineers disconnected from the original project), the hard problem of naming things becomes real. I find that it's useful to keep context. Even though one can never predict with certainty, sometimes you can be real confident that some code is prototype that will be thrown away in six months. And there's a big difference between the code that makes up an API layer and the code that implements a feature constrained to one module.

arp242 3y ago
At not-Google-scale code also often lasts for years; perhaps even more so since there's typically a lot fewer people maintaining it. I think the key thing is to ask yourself "is this really objectively better yes/no?" before commenting. Not that you can never comment if the answer to that is "no", but quite a lot of the time when the answer is "no" it doesn't really matter and it's just "I would have done it slightly different, but both are fine".
- shadowgovt 3y ago
  Good point. Making renames a suggestion, not gating (unless it contradicts the name in some design doc somewhere that another team is relying on not to move) may be the balance point there.
  - arp242 3y ago
    But "another team is relying on not to move" is an objective point, right? Things like "I find this code very hard to follow, and I think it could be made easier" is also objective, and even "I don't understand what this variable name means, and I think it could be clearer". I once names a function mkdir(). This created a directory tree. In the review it was called "obscure" so it became createDir(). Then someone pointed out that "dir" was a needless abbreviation so it became createDirectory(). Then yet someone else pointed out that it's actually recursive, and a discussion on the merits of createDirectoryRecursively() vs. createRecursiveDirectory() was inflicted on everyone. In-between there was a side-quest about directory vs. folder. Just fucking stick a fork in my eye already. Was anything of any objective value gained? I'm having a hard time seeing it. Well, it makes for a slightly amusing anecdote so there's that.
    - ryandrake 3y ago
      I'd wonder what the rest of that API surface looked like. If the rest of the functions were longDescriptiveCamelCase() and then you tried to slip in mkdir(), then I'd say yes, "stylistic consistency" was gained during that unnecessarily difficult back-and-forth. If the other functions were chdir(), remove(), rmdir() and so on, and somehow the reviewers picked your code change as the time to change it all, then yea, what a waste.
      - arp242 3y ago
        It was a mix of short and long and there wasn't really much "style". The company had taken over the maintainership from someone else at some point (before I joined) and it was all fairly inconsistent. I don't really mind the inconsistencies as such, it's just the argueing over nothing that I mind. This kind of stuff was typical. At some point there was a lengthy discussion which prevented rolling out a rather important hotfix over: while (true) { if (someCond()) break; if (otherCond()) break; [..] } It wasn't even about whether someCond() and otherCond() should be in that while(..) condition, it was allegedly "dangerous" because it "could loop infinitely". Well, ehh, that's the case with any lop innit? That's kind of how they work? There was a bunch of instances of code like this, "but it's still dangerous". Hmkay... Oh, and then there was the great "evil incident". I had rewritten much of the frontend to this newfangled thing called "jQuery" that was all the rage. That was all fine and worked pretty well, but suddenly there was a bug hubhub about the jslint comment; the keyword to allow eval() was "evil" (one of those not-too-funny Crockford jokes), so at the top of /static/js/app.minified.js?v=12311 in prodiction there was something like: /*! jslint: keyword1 evil keyword2 */ /*! MIT license blah blah */ minified_js()... And people were up in arms and there was a big panic deploy during lunchtime for this "because customers might see this, and it's highly unprofessional, and it's a big problem we need to fix ASAP" etc. etc. etc. This was in the Netherlands with regular people, not some highly conservative part of the world. It was downright surreal and bizarre. What I'm trying to say is that these people were rather obsessed with minor details to a point I've never seen before or since. Hell, there was a Company Blessed IDE™ that you had to use. Nothing else allowed. It was a complete piece of shit (IMHO) and after a few weeks I just used Vim. It worked. I got stuff done. No one was bothered by it. Still got comments I shouldn't be doing that... While I didn't formulate "is this really objectively better yes/no?" clearly at that time, I'm sure it's been a pretty big influence.

arp242 3y ago

At not-Google-scale code also often lasts for years; perhaps even more so since there's typically a lot fewer people maintaining it. I think the key thing is to ask yourself "is this really objectively better yes/no?" before commenting. Not that you can never comment if the answer to that is "no", but quite a lot of the time when the answer is "no" it doesn't really matter and it's just "I would have done it slightly different, but both are fine".

shadowgovt 3y ago
Good point. Making renames a suggestion, not gating (unless it contradicts the name in some design doc somewhere that another team is relying on not to move) may be the balance point there.
- arp242 3y ago
  But "another team is relying on not to move" is an objective point, right? Things like "I find this code very hard to follow, and I think it could be made easier" is also objective, and even "I don't understand what this variable name means, and I think it could be clearer". I once names a function mkdir(). This created a directory tree. In the review it was called "obscure" so it became createDir(). Then someone pointed out that "dir" was a needless abbreviation so it became createDirectory(). Then yet someone else pointed out that it's actually recursive, and a discussion on the merits of createDirectoryRecursively() vs. createRecursiveDirectory() was inflicted on everyone. In-between there was a side-quest about directory vs. folder. Just fucking stick a fork in my eye already. Was anything of any objective value gained? I'm having a hard time seeing it. Well, it makes for a slightly amusing anecdote so there's that.
  - ryandrake 3y ago
    I'd wonder what the rest of that API surface looked like. If the rest of the functions were longDescriptiveCamelCase() and then you tried to slip in mkdir(), then I'd say yes, "stylistic consistency" was gained during that unnecessarily difficult back-and-forth. If the other functions were chdir(), remove(), rmdir() and so on, and somehow the reviewers picked your code change as the time to change it all, then yea, what a waste.
    - arp242 3y ago
      It was a mix of short and long and there wasn't really much "style". The company had taken over the maintainership from someone else at some point (before I joined) and it was all fairly inconsistent. I don't really mind the inconsistencies as such, it's just the argueing over nothing that I mind. This kind of stuff was typical. At some point there was a lengthy discussion which prevented rolling out a rather important hotfix over: while (true) { if (someCond()) break; if (otherCond()) break; [..] } It wasn't even about whether someCond() and otherCond() should be in that while(..) condition, it was allegedly "dangerous" because it "could loop infinitely". Well, ehh, that's the case with any lop innit? That's kind of how they work? There was a bunch of instances of code like this, "but it's still dangerous". Hmkay... Oh, and then there was the great "evil incident". I had rewritten much of the frontend to this newfangled thing called "jQuery" that was all the rage. That was all fine and worked pretty well, but suddenly there was a bug hubhub about the jslint comment; the keyword to allow eval() was "evil" (one of those not-too-funny Crockford jokes), so at the top of /static/js/app.minified.js?v=12311 in prodiction there was something like: /*! jslint: keyword1 evil keyword2 */ /*! MIT license blah blah */ minified_js()... And people were up in arms and there was a big panic deploy during lunchtime for this "because customers might see this, and it's highly unprofessional, and it's a big problem we need to fix ASAP" etc. etc. etc. This was in the Netherlands with regular people, not some highly conservative part of the world. It was downright surreal and bizarre. What I'm trying to say is that these people were rather obsessed with minor details to a point I've never seen before or since. Hell, there was a Company Blessed IDE™ that you had to use. Nothing else allowed. It was a complete piece of shit (IMHO) and after a few weeks I just used Vim. It worked. I got stuff done. No one was bothered by it. Still got comments I shouldn't be doing that... While I didn't formulate "is this really objectively better yes/no?" clearly at that time, I'm sure it's been a pretty big influence.