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

101–110 of 212 posts

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

#101

Post by Joker_vD »

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…

    const int backupIntervalHours = 24
    const int approxBackupDurationHours = 2
    const int wiggleRoomHours = 2
    const int ageAlertThresholdHours = backupIntervalHours + approxBackupDurationHours + wiggleRoomHours;

    static_assert(28 == ageAlertThresholdHours);
It's a shame more languages don't have static asserts... faking it with mismatched dimensions of array literal/duplicate keys in map literals is way too ugly and distracting from the intent.

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

#102

Post by lucketone »

awesan wrote: post #2

I feel like no one serious uses the uncle Bob style of programming anymore (where each line is extracted into its own method). This was a thing for a while but anyone who's tried to fix bugs in a codebase like that knows exactly what this article is talking about. It's a constant frustration of pressing the "go to definition" key over and over, and going back and forth between separate pieces that run in sequence. I…

It was written in different times, different audiences. (When variable names t,p,lu were the norm)

It was useful for me and many others, though I never took such (any?) advice literally (even if the author meant it)

Based on other books, discussions, advice and experience, I choose to remember (tell colleagues) it as “long(e.g. multipage) functions are bad”.

I assume CS graduates know better now, because it became common knowledge in the field.

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

#103

Post by commandersaki »

antirez wrote: post #97

I made a point here https://antirez.com/news/124 that comments are needed at the same time for different reasons, and different comments have differente semantical properties that can be classified in classes you very easily find again and again, even in very different code bases.

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 longest prefix length
     * to shortest. This is because the ipv6 address is a sequence of bytes,
     * and we want to perturb the address iteratively to get the corresponding
     * network address without making a copy for each perturbation as that
     * would be expensive.
     *
     * For example take address: abcd:abcd:abcd:abcd:abcd:abcd:abcd:abcd.
     *
     * With masks /112, /64, /8 we want to create the following network addresses
     * to lookup as follows:
     *
     * Lookup abcd:abcd:abcd:abcd:abcd:abcd:abcd:0000 in /112 bucket
     * Lookup abcd:abcd:abcd:abcd:0000:0000:0000:0000 in /64 bucket
     * Lookup abcd:0000:0000:0000:0000:0000:0000:0000 in /8 bucket
     *
     * In any other order aside from most specific to least, we'd have
     * to create copies of the original address and apply the mask each
     * time to get each network address; whereas in this case we can take
     * the same address and clear lower bits to higher bits as we go from
     * most specific to least specific masks without incurring any copies.
     */
    for (auto it = m_v6_prefixes.crbegin(); it != m_v6_prefixes.crend(); ++it)
Or here is another for masking a v4 address, but also explaining why a uint64 is used (this is calculated in a hot loop [same as the previous comment example], so I felt it was imperative to explain what is going on as there is very little room otherwise to optimise):

    for (const auto & [ mask_len, bucket ] : m_v4_prefixes)
    {
        /*
         * Example:
         *
         *   netmask 255.255.128.0 (/17) or 0xffff8000:
         *
         *   0xffffffff ffffffff

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

#105

Post by mannykannot »

zimpenfish wrote: post #44
awesan wrote: post #2

I feel like no one serious uses the uncle Bob style of programming anymore (where each line is extracted into its own method). This was a thing for a while but anyone who's tried to fix bugs in a codebase like that knows exactly what this article is talking about. It's a constant frustration of pressing the "go to definition" key over and over, and going back and forth between separate pieces that run in sequence. I…

> 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.

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

#106

Post by broken-kebab »

BoorishBears wrote: post #65
broken-kebab wrote: post #53

IMO the example shows exactly that splitting code in smaller pieces is way better than just commenting it. It makes it easier for dev's brain to parse the code e.g. to understand what code really does , while fattier but commented version makes it harder but tries to replace it with information about original coder's intentions. Which is maybe important too but not as important as code itself. Not to forget that it's…

Splitting example is way too much indirection, but capturing what the code does in the code itself is a preference for me. In any high level language don't know why the middleground wasn't explored: var hasSymbol = getSymbol(symbolName) != null var replacementPending = !alreadyReplaced.contains(symbolName) if(hasSymbol && replacementPending){ alreadyReplaced.add(symbolName); stringToReplace = stringToReplace.replace(…

I like your version, and it's certainly possible to split too much without any practical result just for the dogma. But wrt the particular example I can see what's going on with a glance over the split part, while I have to focus at the commented one. Comments themselves can be helpful, but they can also be misleading cause code and coder's thoughts are not guaranteed to be in harmony all the time.

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

#107

Post by ekjhgkejhgk »

awesan wrote: post #2

I feel like no one serious uses the uncle Bob style of programming anymore (where each line is extracted into its own method). This was a thing for a while but anyone who's tried to fix bugs in a codebase like that knows exactly what this article is talking about. It's a constant frustration of pressing the "go to definition" key over and over, and going back and forth between separate pieces that run in sequence. I…

Great, that's exactly how I feel with any style that demands "each class in its own file" or "each function in its own file" or whatever. I'd rather have everything I need in front of my eyes as much as possible, rather than have it all over the place just to conform with an arbitrary requirement.

I said this at a company I worked and got made fun of because "it's so much more organized". My take away is that the average person has zero ability to think critically.

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

#108

Post by ivanjermakov »

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(quadVel))
        const dragForce = v3.mulScalar(dragDir, dragMag)
        const dragImpulse = v3.mulScalar(dragForce, dt)
        quad.applyImpulse(vec3TwglToRapier(dragImpulse), true)
    }
This way code gets more natural language anchors which helps understanding what it does.

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

#109

Post by necrotic_comp »

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 ?

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

#110

Post by filoeleven »

The bigger point I take from this is that the purpose of comments and good names are all attempts to help the developer grasp "the context of this code". The article uses "context switch" repeatedly, and in fact never uses the word "context" any other way. Since the author acknowledged they're starting a friendly flame war, I'll go ahead and add that the biggest problem with the example code is that it's object-oriented and mutable, which forces a sprawling context on the developers working with it.

When I read the replace() method, I was immediately confused because it has no arguments. stringToReplace and alreadyReplaced are properties of the class, so you must look elsewhere (meaning, outside of the function) for their definitions. You also don't know what other bits of the class are doing with those properties when you're not looking. Both of these facts inflate the context you have to carry around in your head.

Why is this a class? In the replace() method, there is a call to translate(symbolName). Why isn't there also a SymbolNameTranslator class with a translate() method? Who decided one was simple enough to use a function while the other warrants a class?

SymbolReplacer surely could also be done with a function. I understand that this is illustration code, so the usage and purpose is not clear (and the original Bob Martin article does not help). Is there a reason we want these half-replaced strings made available every time we call SymbolReplacer.replace()? If there is, we can get the same data by using a reduce function and returning all of the iterations in a list.

A plain, immutable function necessarily contains within it the entire scope of its behavior. It accepts and returns plain data that has no baggage attached to it about the data's purpose. It does one thing only.

Post reply on HN