Hacker News Reader

Comment by bluGill | Hacker News Reader

What is wrong with

   time++;

That seem obvious enough to me without any comments.

View on HN

Is it ms? seconds? days? weeks? months? How far up do I have to read to figure that out?

When I'm looking at a test case is broken, I ideally want context IN the actual test that lets me understand what the test author was thinking when they wrote it. Why does this test exist as it does? Why are the expectations that are in place valid? Write the comments for you-in-2-years.

That just means the variable isn't named correctly, not that it needs a comment. Just name it 'time_seconds" or whatever and save yourself the extraneous typing.

I tend to be a minimalist when writing comments. If I have to write out a comment to describe what I'm doing (like "advance 1 simulated second"), then I have failed at writing clear code. Sometimes I will write a comment to explain why I am doing something, if it's not clear (like "manually advance time to work around frobbing bug in foobar").

Comments add to your maintenance burden. Writing clearer code can often reduce that burden.

Do you not know the conventions of your project? Doesn't your project have a convention that all time is in ms (second, weeks...)?

If your project doesn't have that convention such that everyone knows than the code should be

timeMs++;

You may also have a time type and so you can use your IDE to examine the type.

I agree that the time unit should be in the variable name. The code itself should do a good job of explaining "what" is happening, but you generally need comments to explain "why" this code exists. Why is the test advancing the time, and why are we advancing the time at this line of the test?

    networkTimeMs++; // Callback occurs after timeout

    timeSec++; // Advance time to check whether dependent properties update

    utcTime++; // Leap second, DON'T advance ntpTime

> I agree that the time unit should be in the variable name

Also a terrible solution!

The code suffers from primitive obsession. Unless you're in a code section that is known to have performance issues, use real types.

    time = time.plusMilliseconds(1);

In a performance language your "real types" aren't somehow more expensive and so you should only use the built-in primitives when they accurately reflect your intent. So I would write:

time += Duration::from_millis(1);

But I would expect that "time unit should be in the variable name" is a reasonable choice in a language which doesn't have this affordance, and I needn't care about performance because apparently the language doesn't either.

I also wonder why we've named this variable "time". Maybe we're a very abstract piece of software and so we know nothing more specific? I would prefer to name it e.g. "timeout" or "flight" or "exam_finishes" if we know why we care about this.

> Do you not know the conventions of your project?

I just started yesterday! No, I don't.

I would prefer `somethingSec`, where "something" indicates the usage better than "time". E.g. `delaySec` or `elapsedSec`.

Same here - unit-specifying variable names like "delaySecs" and "amountBaseCcy" (where any possibility of ambiguity exists) are exactly what I enforce on our projects (when types aren't providing the guarantee). It makes avoiding and detecting mistakes easier, because you can immediately see where logic has gone wrong.

Pedantic but a comment clarifying the unit of measurement belongs with the declaration of the variable, not an increment statement.

The problem, in this case, is that the correct size of the increment involves the unit of measurement. If we change the unit of measurement and go update your comment on the declaration of the variable, now everywhere which uses the variable is wrong.

    int time; // in seconds
    /* thousands of lines away or in another file */
    time += 1;

Later we change the time to be in milliseconds. We update the comment on the declaration, but now that code is wrong and we have no reason to know that.

That's a bad choice, languages should do better (and some do - where they do, use the better features and this problem vanishes) but when it's forced upon us it makes sense to either put the unit in the name of the variable or ensure comments about changes to the variable explain the units consistently, even though that's lots of work. This extra work was dumped on you by the language.

That code (in isolation) does not tell me what unit time is though

It will vary immensely on how readable the actual code base it, but what comes to mind:

1. what units? I was just caught in this with a function with a timeout. I had to look at the docs to find out this was actually in nanoseconds (stuff like this is why I came more around to verbose parameter names).

2. what's the function of the timer?

3. (potential code smell) Do I need to manually increment such a timer for the test? is the time library a necessary part of the test (or perhaps what we testing)?

Advance a frame, a second, or a millisecond?

Reply on news.ycombinator.com