Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Maybe comments should explain 'what' (2017)

hillelwayne.com

Post reply on HN
Print view View original

111–120 of 212 posts

Re: Maybe comments should explain 'what' (2017)

#111

Post by scottlamb »

This is tangential to the article's point, but that `replace` function is a complete WTF in a way both authors completely ignore. Because it replaces things in the entire string in a loop, it will translate symbols recursively or not depending on ordering. Imagine you have the following dictionary:

    a=$b
    b=oops
if your input string just has one of these, it will just be translated once as the programmer was probably expecting:

    input:  foo $a bar
    output: foo $b bar
but if your input string first references $b later, then it will recursively translate $a.

    input:  foo $a bar $b
    output: foo oops bar oops
Sometimes translating recursively is a bizarre behavior and possibly a security hole.

The sane thing would be to loop through building the output string, adding the replacement for each symbol as you go. Using String.replace and the alreadyReplaced map is just a bad idea. Also inefficient, as it and throws away strings and does a redundant search on each loop iteration.

Feels typical of this whole '90s-era culture of arguing over refactoring with Java design patterns and ornate styles without ever thinking about if the algorithm is any good.

Edit: also, consider $foo $foobar. It doesn't properly tokenize on replacement, so this will also be wrong.

Re: Maybe comments should explain 'what' (2017)

#112

Post by shiandow »

If we're doing hot takes then I propose the following guideline taken from mathematics.

Only comments should explain what, variable names should only hint

The first example is perfectly fine, nobody has the time to derive or read a verbose formula involving the words 'weight', 'radius' and 'price'.

Re: Maybe comments should explain 'what' (2017)

#113

Post by codemonkey-zeta »

ivanjermakov wrote: post #108

I noticed that when I write code that is not trivial to understand I tend to extract intermediate values into variables with meaningful names. applyDrag(): void { const { quad: quadConfig } = settings const quad = this.getRigidBody() const quadVel = vec3ToTwgl(quad.linvel()) const dragMag = aerodynamicDrag(quadConfig.dragCoefficient, v3.length(quadVel), quadConfig.frontalArea) const dragDir = v3.negate(v3.normalize(q…

I find that Lisps encourage this behavior more than other languages. Many (most?) of the lisp functions I read have the exact same structure:

  (fn some-function [args]
    (let [binding (transform args)]
      (an-expression binding)))
I find that this makes skimming lisp code much easier, because I can usually skip reading the bindings and just read the function name and ultimate expression and usually get the gist very quickly.

You might wonder how this is different than the example you provided, and the answer is because you could sneakily intersperse anything you wanted between your imperative bindings (like a conditional return statement), so I actually have to read every line of your code, vs in a lisp `let` I know for a fact there is nothing sneaky going on.

Re: Maybe comments should explain 'what' (2017)

#114

Post by shiandow »

scottlamb wrote: post #111

This is tangential to the article's point, but that `replace` function is a complete WTF in a way both authors completely ignore. Because it replaces things in the entire string in a loop, it will translate symbols recursively or not depending on ordering. Imagine you have the following dictionary: a=$b b=oops if your input string just has one of these, it will just be translated once as the programmer was probably e…

The insane thing to do would be to implement a variant of loeb so it works regardless of order.

Re: Maybe comments should explain 'what' (2017)

#115

Post by tangotaylor »

“What” comments can be quite nice to quickly parse through code.

But I don’t think they’re worth it. My issue with “what” comments is they’re brittle and can easily go out of sync with the code they’re describing. There’s no automation like type checking or unit tests that can enforce that comments stay accurate to the code they describe. Maybe LLMs can check this but in my experience they miss a lot of things.

When “what” comments go out of sync with the code, they spread misinformation and confusion. This is worse than no comments at all so I don’t think they’re worth it.

“Why” comments tend to be more stable and orthogonal to the implementation.

Re: Maybe comments should explain 'what' (2017)

#116

Post by ivanjermakov »

necrotic_comp wrote: post #109
ivanjermakov wrote: post #108

I noticed that when I write code that is not trivial to understand I tend to extract intermediate values into variables with meaningful names. applyDrag(): void { const { quad: quadConfig } = settings const quad = this.getRigidBody() const quadVel = vec3ToTwgl(quad.linvel()) const dragMag = aerodynamicDrag(quadConfig.dragCoefficient, v3.length(quadVel), quadConfig.frontalArea) const dragDir = v3.negate(v3.normalize(q…

I appreciate this way of programming - also, if I may, in the age of auto-complete I think it's okay to have verbose variable naming. Imho, it's perfectly fine to have quad, quadVelocity, dragMagnitude, etc. I see this a lot in the wild, though - as an honest question (not trolling!) why do people still shorten their variable names in place of having a terse descriptor ?

I tend to have shorter names for function scoped variables. Wider the scope - more descriptive the name. Short names are good to have more concise code and more logic fitting into a line width.

Re: Maybe comments should explain 'what' (2017)

#117

Post by ivanjermakov »

codemonkey-zeta wrote: post #113
ivanjermakov wrote: post #108

I noticed that when I write code that is not trivial to understand I tend to extract intermediate values into variables with meaningful names. applyDrag(): void { const { quad: quadConfig } = settings const quad = this.getRigidBody() const quadVel = vec3ToTwgl(quad.linvel()) const dragMag = aerodynamicDrag(quadConfig.dragCoefficient, v3.length(quadVel), quadConfig.frontalArea) const dragDir = v3.negate(v3.normalize(q…

I find that Lisps encourage this behavior more than other languages. Many (most?) of the lisp functions I read have the exact same structure: (fn some-function [args] (let [binding (transform args)] (an-expression binding))) I find that this makes skimming lisp code much easier, because I can usually skip reading the bindings and just read the function name and ultimate expression and usually get the gist very quickl…

It's the same for many FP/pure languages, where function is a single expression.

Re: Maybe comments should explain 'what' (2017)

#118

Post by lukan »

epgui wrote: post #99
jackfranklyn wrote: post #10

[flagged]

What you describe really is describing the "why", not the "what". The line between the two is not that blurry: assume your reader has total knowledge of programming, and no knowledge whatsoever of the outside world. Comments about what the code does to bits are the "what"; comments about how the code relates to the outside world are the "why". The rest is a matter of taste and judgment.

Just curious, you advice against "what" comments?

"assume your reader has total knowledge of programming"

Because if I know my fellow programmers have like me not a total knowledge of programming, what comments before footguns seem useful to me.

Or when there was a hack that is not obvious.

To me it mostly is not a question of taste, but context. Who will read the code?

Re: Maybe comments should explain 'what' (2017)

#120

Post by tetha »

dijksterhuis wrote: post #57
tetha wrote: post #50

Earlier quoted context omitted.

> That's explaining "what" but also implicitly "why" - because that's how double-entry works and that's the tolerance banks allow for settlement delays. You can't really extract that into a method name without it becoming absurd. That's why I've also started to explicitly decompose constants if possible. Something like `ageAlertThresholdHours = backupIntervalHours + approxBackupDurationHours + wiggleRoomHours`. Sure,…

i personally prefer this kind of version — if i want to do the maths to work out tweaks i can, but i’m not forced to do maths in my head to know/tweak the end value // a total of // - backup interval = 24 // - approx backup duration = 2 // - “wiggle room” = 2 ageAlertThresholdHours = 28 yes lazy devs are lazy and won’t want to or just won’t update the comments (be pedantic in review :shrug:). it’s all trading one thi…

> sorry, i’ve basically done an unprompted code review. i feel like a bit of a dick now.

That's all fine.

Just note that this was one of the easiest examples I could find. For example, for reasons out of my control, the individual network configuration on a linux host is positively nuts. The decision whether to deploy routes, static DNS servers and such depends on 3-5 facts about the datacenter and the provider it's on.

In such a case, it is more maintainable to separate the facts about the provider, or the thing we are looking at (e.g. "Does this provider allow us to configure routes in their DHCP server?", from the computation/decision making ("Can the system rely on the routes from the DHCP servers?"), and all of that from the actual action based off of the decision ("Deploy routes statically if DHCP provided routes are not correct").

Post reply on HN