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.

closed as primarily opinion-based by user22815, jwenting, Thomas Owens Mar 22 '17 at 15:17

Many good questions generate some degree of opinion based on expert experience, but answers to this question will tend to be almost entirely based on opinions, rather than facts, references, or specific expertise. If this question can be reworded to fit the rules in the help center, please edit the question.

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

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

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.

Not the answer you're looking for? Browse other questions tagged or ask your own question.