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

151–160 of 212 posts

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

#151

Post by peheje »

tetha wrote: post #141
peheje wrote: post #130

Earlier quoted context omitted.

I don't necessarily disagree with providing context, but my concern is that comments eventually lie. If the business rule evolves (say the window moves to 5 days) the comment becomes a liability the moment someone updates the code but forgets the prose. The comment also leaves me with more questions: how do you handle multiple identical amounts in that window? I would still have to read the implementation to be sure.…

This however misses an important point: 3 is not in our control. 3 in general is controlled by math-people, and that 3 in particular is probably in the hands of a legal/regulation department. That's a much more important information to highlight. For example, at my last job, we shoved all constants managed by the balancing teams into a static class called BalancingTeam, to make it obvious that these values are not in…

I like the idea of using a LegalConstants namespace or Code Owners to signal that we don't own those values.

However, I’d argue that being 'out of our control' is actually the main reason to test them. We treat these as acceptance tests. The goal isn't flexibility, it is safety. If a PR changes a regulated value, the test failure acts as a tripwire. It forces us to confirm with the PM (and the Jira ticket) that the change is intentional before merging. It catches what code structure alone might miss.

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

#152

Post by mgkimsal »

jackfranklyn wrote: post #10

[flagged]

The 'comment becomes a lie' is because you've got magic number there in the comment.

If the comment was

  // We match them by looking for equal-opposite amounts within an X-day window defined by BANK_TRANSFER_MATCH_WINDOW_DAYS
the comment is more evergreen, until the actual logic changes. If/when the logic changes... update the comment?

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

#153

Post by zahlman »

jackfranklyn wrote: post #10

[flagged]

> The Uncle Bob approach of extractNameMatchingTransfersWithinSettlementWindow() doesn't actually help

Sure it does. You've isolated the "what" to the name, and the comments can focus on the "why".

(Although I think some of the XP advocates would go off the rails and try to model a settlement window as an object....)

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

#154

Post by nooee »

Also note, that after Uncle Bob's refactoring, you now have six additional functions floating around, which may or may not make sense outside of the context of their original caller. This can make the API surface of your class (even if it's just the internal one) harder to grasp and it invites other devs to reuse code that was never intended for reuse, creating unintended couplings. That's mainly a problem for languages that don't allow nested functions though. But in Java, I am sceptical of excessive function creation for the sake of self-documenting code, unless maybe it's in the context of a command pattern, where the whole class only has one obvious function anyway.

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

#155

Post by epgui »

lukan wrote: post #118
epgui wrote: post #99

Earlier quoted context omitted.

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?

I almost (?) always advise against “what” comments. I have rarely (if ever?) encountered any cases where “what” comments didn’t have a better (and practical/cheap/easy enough) solution.

In my experience, when I review junior contributors’ code and see “what” comments, it’s usually caused by 1) bad naming, or 2) abstractions that don’t make sense, or 3) someone trying to reinvent maths but incorrectly, or 4) missing tests, or 5) issues with requirement gathering / problem specification, or 6) outright laziness where the contributor doesn’t want to take the time to really think things through, or 7) unclear and overcomplicated code, or… any number of similar things.

At the very least, any time you see a “what” comment, it’s valuable to take notice and try really hard to think about whether the problem the comment tries to solve has a better solution. Take it as a personal challenge.

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

#156

Post by zimpenfish »

mannykannot wrote: post #105
zimpenfish wrote: post #44

Earlier quoted context omitted.

> I feel like no one serious uses the uncle Bob style of programming anymore (where each line is extracted into its own method) Alas, there's a lot of Go people who enjoy that kind of thing (flashback to when I was looking at an interface calling an interface calling an interface calling an interface through 8 files ... which ended up in basically "set this cipher key" and y'know, it could just have been at the top.)

Hardcore proponents of this style often incant 'DRY' and talk about reuse, but in most cases, this reuse seems to be much more made available in principle than found useful in practice.

There's also the "it makes testing easier because you can just swap in another interface and you don't need mocks" argument - sure but half of the stuff I find like this doesn't even have tests and you still tend to need mocks for a whole bunch of other cases anyway.

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

#157

Post by xg15 »

The "don't explain variable definitions with comments because people won't see the comments at usage sites" argument also seems obsolete in the face of modern(?) IDEs.

If I look through code and see a variable I don't know, I want to see its definition anyway, so I know the type, scope, initial value, etc. And it's trivially possible to do that with everything that has a "jump to definition" command.

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

#158

Post by lukan »

epgui wrote: post #155
lukan wrote: post #118

Earlier quoted context omitted.

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?

I almost (?) always advise against “what” comments. I have rarely (if ever?) encountered any cases where “what” comments didn’t have a better (and practical/cheap/easy enough) solution. In my experience, when I review junior contributors’ code and see “what” comments, it’s usually caused by 1) bad naming, or 2) abstractions that don’t make sense, or 3) someone trying to reinvent maths but incorrectly, or 4) missing t…

For sure, bad code exists. But if I have to work with bad unclear code, "what" comments are very helpful.

Like something really bad

x=y //triggers method xyz

So I would agree that under controlled conditions, they should not be necessary.

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

#159

Post by panstromek »

mgaunard wrote: post #94
panstromek wrote: post #49

Earlier quoted context omitted.

Sure, but you can't always fix the bug if it's not in your system.

Fork it, you should have ownership of your whole stack. If you have the spare time, you can try and submit your patches upstream; in the meantime, you just maintain your own version.

No, you can't always do that. We have workarounds for platform bugs that were even fixed, because we get users with old devices that can't upgrade. You cannot fork a phone of a random person on the other side of the world. Once a platform bug is out, it can stay out in the wild for a very long time.

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

#160

Post by commandersaki »

Karliss wrote: post #150
commandersaki wrote: post #103

Earlier quoted context omitted.

This is a great post and meshes with how I like to comment as well. I like to break the so called rules and get a bit dirty when it comes to writing code and comments. My opinion which you state, is to remove the effort from the reader in needing to figure things out a second, third, or n-th time. Here is one I wrote just to talk about iterating a loop in reverse: /* * We iterate the v6 prefixes in reverse from longe…

Except your giant comment doesn't actually explain why it used uint64. Only place mentioning uint64 is integer promotion which only happens because you used 64bit integer, thus no explanation of why. Was it done because shifting by amount equal or greater to integer width is undefined behavior? That would still not require storing result in 64bit mask, just shifting (~0ULL) would be enough. That would be a lot more v…

Sorry, I didn't explain uint64 was used. I wrote this many years ago so my memory was foggy, but I went through a few iterations using uint32 only to use branches for the masks. This was the only branchless way I could come up with at the time after a few attempts. I think the example was more to demonstrate that the algorithm was correct and I wasn't going to unit test it at that scope.

As for the 128-bit addresses, we used boost ip::address_v6 to_bytes() as it appears there was no masking option.

For my taste it would be sufficient to say "Iterate in reverse order from most specific address to least specific. That way address can be a calculated in place by incrementally clearing lowest bits." Having 2 paragraphs of text which repeat the same idea in different words is more distracting than it helps.

Ah apologies, too late now, should've mentioned it in the PR. But I expected it would ruffle some feathers, I don't care for conventions or other people's quibbles. As long as it improves understanding for the reader, regardless of how "redundant", then mission accomplished.

Post reply on HN