Hacker News Reader

Comment by clarry | Hacker News Reader

Most code should be boring and immediately obvious. I don't run into lots of real world code that needs much explaining. Sometimes you do something weird or not-obvious for good reason, but I see plenty of comments explaining things that only need to be explained due to poor design. Also poor comments that don't really explain anything that isn't obvious by looking.

View on HN

    def clean_start_year(x):
        # Account for the 7 year offset in the database
        return x + 7

    def process_record(record):
        record['start year'] = clean_start_year(record['record'])

It's perfectly obvious what this code does, but without the comment it's totally unclear why it does what it does.

   const database_year_offset = 7;

   fix_start_year(year):
       return year + database_year_offset;

   process_record(record):
       record['start year'] = fix_start_year(record['record']);

In this version with one less magic constant (and renamed function), the comment would look very redundant.

DougBTX had the same idea, at the same time. I don't think their version needs the comment either. EDIT: Dang you removed a perfectly fine post :)

I guess, but I think it's more that my example fails to illustrate the point well.

Informative comments cannot always be "factored out" like this. Do you at least agree that a ticket or bug #, or a link to an issue in an issue tracker, would be appropriate?

I think a comment is appropriate if the problem cannot be factored out. But in my experience such things can be factored out most of the time, hence the argument that most code should be boring and immediately obvious.

Factoring out a comment into code sometimes loses information. I would argue that database_year_offset is not as informative as the original comment. Free form text can say things better than identifier names.

Most code, sure. But it's not an ideal to be striven for, or a metric to be used when evaluating code.

Ah, the inline equivalent of Doxygen-for-the-sake-of-Doxygen documentation.

    int frob(const int x, void *context)
    Performs a frob on x in the given context.
    Parameters:
      x - int param
      context - a pointer to context
    Returns an id of the frob performed.

I see a lot of this at work, sigh.

The better a variable / function is named (and the better its scope or purpose is limited to one thing), the less it needs comments. So there are a lot of self explanatory variables or functions with doxygen comments that state exactly the same thing the name already does, without adding anything besides noise. And then there are some poorly named things where the exact same thing happens: comment re-states what the bad identifier states. Sigh. Genuinely useful comments are rather rare.

How is what I posted an example of this, in any way?

Use your imagination. The "cleaning" routine might not be a one-liner. Maybe it depends on multiple functions from another file/package/library.

The comment doesn't explain why there even is an offset in the first place. Who put it there and why? I think this should be explained as well

The code might be obvious, but if you don't include the reason why it exists it's a target for removal. And having too many of those makes either for garbage or hard decisions when refactoring.

Writing the rationale is the most important comment you must not skip.

And there would be links to design documentation so that it can be kept up to date. (For non-programmers.)

Why it exists ought to be explained by the existence of a test.

This isn't always possible, but it's far more possible than many developers seem to think.

That just makes two things that need to be commented doesn't it?

When I read code I care about what the code is supposed to do and why it's supposed to do that.

There is code that passes the current tests and is logically sound but no longer fits the business requirements. Knowing that it was implemented to solve a certain use case helps the reader / reviewer see whether the code and the test still fit or if they should be updated.

The best comments I've come across comment the intention and any context that isn't immediately obvious from the function.

Your test should explain what it's testing. Via it's name or some other mechanism. Otherwise how do you know what's gone wrong when it fails?

That other mechanism is comments.

Does that explain why, though? It explains that it's there, and someone obviously wanted it to be, but it still doesn't get at the why. I've found that most of the logic I'm left scratching my head at is due to some business rules that there's no better way to deal with - even if a test exists for it I still can't figure out what the tests purpose was, other than to just verify that it's working.

Just yesterday I wrote some code that was pretty boring, pretty readily understandable, and also pretty apparently a good target for an easy simplification. However that obvious simplification would also non-obviously introduce a bug that might not be noticed immediately. So I left a comment to the effect of "yeah these two lines look kinda dumb but we need it because X"

Could I have instead massively restructured the feature to prevent the possibility of the bug the comment was explaining the workaround for? Probably. But that would have increased the number of lines of code to make the change by a factor of something like 20x-200x and would have required far more testing. It also probably would have resulted in something overall more complicated.

I'm not sure whether I should envy or pity you. Writing code that is that obvious and boring cannot be very fulfilling. How long until a machine does that for you (after all, automating boring and obvious stuff is what we programmers do)? OTOH, on days where I feel lazy I can only wish my world were that simple.

Any monkey can puke thousands of lines of code. Producing code so boring that fellow humans can understand it immediately is the real challenge in programming. It requires much more effort and care than blaming everyone else for not understanding a sophisticated pile of mess.

Reply on news.ycombinator.com