Hacker News new | ask | show | jobs
by seanwilson 1756 days ago
For the first example with conditions, I much prefer rolling the "why" of the comments into boolean variable names where possible e.g.

   // We still have burst budget for *all* tokens requests.
   if self.one_time_burst >= tokens {
      ...
   } else {
      // We still have burst budget for *some* of the tokens requests.
becomes something like (I'm missing context but you get the idea):

   enoughBudgetForAllTokenRequests = self.one_time_burst >= tokens

   if enoughBudgetForAllTokenRequests
      ...
   else
      ...
I don't see this often though. I see the commenting pattern a lot where the comment usually duplicates the "what" of the conditional code, and it's often ambiguous if the comment is talking about the condition or a whole branch.

I think a descriptive variable makes the code easier to skim, it's more precise and less likely to become inaccurate. For a big complex conditional, you can break in up into several well-named boolean variables you compose together (but obviously comment the bits that can't be covered by a concise variable name).

3 comments

Long variable names mean you perceive your simple code as being too complicated. It's an if statement. I would refactor your long variable name to something more compact like enough_budget.
enough_budget for what though? That's not a very clear variable name.
IMO that should be in a comment, instead of duplicating that information every time you mention the variable. (This applies more for local variables in short blocks than for, say, global variables.) At some point the length of the name makes everything around it harder to read, since it’s been pushed so far apart.
As a general rule I completely agree.

On the other hand, token bucket implementations are hard to understand and hard to debug. And an ambiguous variable name can cause trouble because some devs might think it "obviously" refers to enough budget for some other slice of requests.

I think my preferred solution here is to extract the condition into a boolean function with a long name like HasBudgetForAllRequests. This solves the long variable name issue and also gives you a useful place to add to your budget computation logic if you need to.

The words Has and For are unnecessary words in that variable name.

Think of code as mathematical notation. Some degree of familiarity with the domain is expected. You can use shorthand symbols or words as variable names.

Otherwise you're trying to write English sentences into compact notation and that's just noise. Hard to read.

Try read the A* algorithm when the variables have all been expanded out and you'll find it's impossible to see the forest for the trees. Sometimes G is the correct name for the collection of the cheapest scores known so far. It's compact and is best explained with a comment.

total_traffic_budget
I prefer this too, but sometimes poor type-narrowing support hinders it. For example, TypeScript only recently added support for aliased type-narrowing:

https://devblogs.microsoft.com/typescript/announcing-typescr...

This. And the extra documentation is provided by tests, which are living code and evolve together with the production code. That should be enough to understand the code.

The top of the file comment is good and well received. At the same time, you could have a wiki page with that info, where you can also have diagrams etc.