4

Suppose that you see the following pattern in code:

function foo() {
    ...
    var someFlag = false; // Until contested
    for/while/if () {
        ...
        // Possibly deeply nested in some non-trivial logic
        someFlag = true;
        ...
    }
    if (someFlag) {
        // Additional processing of some sort
    }
    ...
}

With no further description, do you have a good idea of what that "until contested" comment is trying to describe? Do you think it would be a helpful comment in understanding code you were reading?

It's describing a boolean flag which starts off as being set to one value, until some later point(s) when it may switch to the other value but, not back once it has been set. Common uses: "found" / "completed" / "did change something" etc...

Do you have any way that you prefer to describe said pattern of code? "Until contested" is the most concise / clear way that I've found to quickly tag it in my own code for ease of reading in cases that don't really warrant a more detailed comment. I'm wondering if it would be a reasonable comment to use in code I write on a team professionally too.

Bent
  • 2,576
  • 1
  • 14
  • 18
Mark
  • 51
  • 3
  • 3
    On my team, expletives would be uttered if this was found in the code. – MetalMikester Mar 10 '17 at 19:17
  • 3
    Contested is (imo) a completely incorrect word here. What you describe is a "latching" toggle. – CPerkins Mar 10 '17 at 19:20
  • @CPerkins I do agree that it's a "latching" toggle, but I've never seen "latching" used to describe anything in software, that comes off to me as an exclusively hardware term. – Mark Mar 10 '17 at 19:22
  • 1
    Fair enough - but "contested" is 100% inappropriate here. There's no "contest" involved, just a one-way switch from one Boolean value to another. I submit that "latching" is a vastly better word. – CPerkins Mar 10 '17 at 19:30
  • The term "latch" is a fairly common english word, and used in many domains (most commonly, the switch on a locked door to prevent the locking mechanism from from shutting). If a plain-english non-software word provides an accurate description to something you're doing in your code, then that's a pretty good starting point for a code. Remember that English comments are meant to be understood by English-speaking humans – Ben Cottrell Mar 10 '17 at 19:37
  • 1
    I don't see the point of the comment. The var is already called a Flag and defaulting to false. – GrandmasterB Mar 10 '17 at 19:48
  • 1
    I can say: for me the comment actually did more harm than good. I substantially misinterpreted the meaning of "contested" here and actually had to read the code a few extra times because the comment made me think incorrectly about what was going on. – Cort Ammon Mar 10 '17 at 20:31
  • @GrandmasterB someFlag is exactly an example of bad choice of name. OP has mentioned some good choice of names in the spoiler section; please mouse over to read it. The name needs to explain its own raison d'etre. someFlag fails to hint strongly that its initial value is not its final value (i.e. it doesn't distinguish itself from a local immutable value). The other good examples, however, strongly hints that they are some values to be determined. Any not-so-silly programmer could reason that the value is going to be computed soon. – rwong Mar 11 '17 at 00:12
  • I would rather state "until proven wrong/different" but it is clear to me. And I am not negative about it. It does add useful knowledge, it says "although we are setting this to false, it may not be the ultimate valid value, it is just a default for now" and you know what will follow is a series of tests to determine the ultimate value. That is a lot of information and helpful guidance. People stating "it is useless, you can tell all that from reading the code" are missing the purpose of comments. Code should not be a puzzle. – Martin Maat Mar 11 '17 at 08:17
  • This statement may become a lot more readable is you replace the meaningless "someFlag" with the actual name of the flag as it is used in the code. – gnasher729 Mar 11 '17 at 15:25
  • @rwong This is SE, not SO. I thought it pretty clear that he wasn't literally naming the variable 'someFlag', but rather using that for illustrative purposes. Clearly someFlag isn't terribly useful if that was an actual variable name. – GrandmasterB Mar 12 '17 at 05:23

3 Answers3

2

A contest involves two or more parties that may be in contention (note the common Latin root contestari). If I were to see this comment I might assume there is some sort of threading contention in effect, or that there is something uncertain about the condition even if the flag is set to true.

I would suggest the following:

  1. The comment should be unnecessary if the variable is properly named. After all, the comment appears only once while the variable is going to be all over the place, apparently. Are you going to comment every reference to it?

  2. The variable should be named in a manner that describes what it is doing. I would offer a suggestion but your question is too general. There is no general name that would work in all cases; what's more, I would avoid trying to think of a general name. Sometimes things should be specific and not general.

  3. If you insist on a general name, flag is adequate, and is certainly general.

John Wu
  • 26,462
  • Flags can change state back and forth. OP says clearly that this is one-way. No reason to throw away that intent. – CPerkins Mar 10 '17 at 20:49
  • Nah. A flag is a signal. The CPU carry flag, for example, can only be set, not cleared, by an arithmetic operation; you need a special operation (CLC) to clear it. If you throw the flag on a play, you cannot unthrow it, although you can roll it back up and put it in your pocket for the next play. This is opposed to words like bit, state, indicator etc, which are more like flip/flops instead of latches. The word is adequate. But as I pointed out in my post, general terms like this should usually be avoided. – John Wu Mar 10 '17 at 20:58
  • Fair enough, though I've seen a lot of use of "flag" in really just a "this is a Boolean" kind of way. Agree that general terms should be avoided. – CPerkins Mar 10 '17 at 20:59
  • After reading through all the comments on the thread and reevaluating what I was doing I think just going with including the extra word "flag" in the variable name itself would be best, I'm going to do that going forward. At first I was thinking that flag could be confusing with people sometimes using that to mean any boolean as @CPerkins notes, but that's not super common. – Mark Mar 13 '17 at 14:55
1

The comment is not helpful. I am one that prefer far more comments than many on this site that prefer code to be selfexplanatory without comments.

In this case the flag variable needs a good descriptive name. No comment needed. No comment would save the situation where the name of the variable is bad.

The code would then be selfexplanatory.

Bent
  • 2,576
  • 1
  • 14
  • 18
0

Comments whose sole purpose is to explicitly describe how the code works are not useful to somebody reading the code because the code itself provides that same information - it's effectively a form of duplication.

The one exception to this situation may be for something naturally hard-to-understand like a highly complex scientific algorithm - in which case a comment probably needs to say something like "This is the foo algorithm" so that the reader can go and read the appropriate requirement or book to understand it.

To avoid the need for comments explaining your code logic, carefully consider the names of your variables and functions; also consider the length and cyclomatic complexity of your functions - maybe your code would benefit from breaking things down into smaller pieces.

Assuming neat/modular/readable code, as a reader or maintainer the information which is most valuable are usually about the types of decisions and trade-offs made by the author, and information the author had in their head at the time which can't be expressed in code simply through naming.

Some contrived examples:

  • "When true, this indicates the system is waiting for authorisation, and prevents the user from pressing cancel"

  • "This flag is published to the system management service every 30 secs, if the system manager doesn't receive the update or gets a wrong state then the sync will fail".

  • "This edge-case only happens when the user has been de-authorised from the server by an administrator but their client app isn't responding to the message forcing them to disconnect."

  • "The ACME FooBar API is restricted to 10 API calls every 5 seconds, so once the count is reached, the thread must sleep for 5 seconds to avoid throttling".

In short, consider the ways in which somebody in the future (maybe even yourself in the future) might accidentally and naively "do the wrong thing" to your code and either end up breaking it, or end up creating some unintentional side-effects because they didn't have the same depth of understanding or 'bigger picture' view as you did when you wrote it.

Ben Cottrell
  • 11,739