Software development topics I've changed my mind on

> time++; // increment time

This isn't too many comments, it's a poor quality comment. Try:

time++; // advance 1 simulated second

What is wrong with

   time++;

That seem obvious enough to me without any comments.

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 code (in isolation) does not tell me what unit time is though

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 would prefer `somethingSec`, where "something" indicates the usage better than "time". E.g. `delaySec` or `elapsedSec`.

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

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

> Do you not know the conventions of your project?

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

> 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);

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)?

Like many comments, I find we can eliminate it without losing anything by using better variable names.

   seconds++;

That gets the idea across very clearly to me and has the benefit of (likely) making the rest of the code clearer too

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.

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.

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.

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.

Advance a frame, a second, or a millisecond?

and the variable will invariably store milliseconds because someone didn't read the docs on timelib.Now() or store an int as a counter for makeshift vector clock :p

types for the win