Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New NL Rule suggestion: Place comment-only lines before the code they apply to #2181

Closed
pmolodo opened this issue Feb 21, 2024 · 3 comments
Closed
Assignees

Comments

@pmolodo
Copy link

pmolodo commented Feb 21, 2024

Argument for having a rule

Placing comments lines before the code they apply to seems to be a de-facto standard, but it's rarely explicitly stated, or explained why it's preferable - so having a rule in a resource like CppCoreGuidelines is useful when doing code reviews.

Also, while the CppCoreGuidelines examples generally prefer in-line comments, when they do employ comment-only lines, this seems to be the rule that they follow - ie:

  • in the suggested replacement of the first example of P.5:

    // Int is an alias used for integers
    static_assert(sizeof(Int) >= 4);    // do: compile-time check
  • in the second example of ES.102:

    // compares signed to unsigned; some compilers warn, but we should not
    for (gsl::index i = 0; i < v.size(); ++i) v[i] = i;
    
    int a2[-2];         // error: negative size
    
    // OK, but the number of ints (4294967294) is so large that we should get an exception
    vector<int> v2(-2);

Example Draft of Rule

NL.?: Place comment-only lines before the code they apply to

Reason C++ code is read top-bottom, and it is generally useful to provide context before reading the code. If placed after, then the reader not may not notice the comment until after they've wasted time trying to understand something the comment clarified. Placing it before allows the reader to skip reading the associated code if they wish. Reduces ambiguity about what line of code a comment is associated with.

Example

// Explanation for (x + y) / z  - good
auto result = (x + y) / z;

auto result2 = z - y * x;
// Explanation for z - y * x - bad

Exception This rule does not apply to in-line comments, or lines which are continuations of in-line comments.

Example

auto result3 = 2*x / z;  // Explanation for 2*x / z (in-line comment)
auto result4 = x + y + z;  // Explanation for this formula, that continues
                           // on another line (continuation of in-line comment)

Exception This rule does not apply to comments not strongly associated with a specific line of code.

Example

string myVar;
// TODO - make sure we blah (no specific code line association)

Exception This rule does not apply to comments specifically associated with the state after a line of code executes.

Example

doSomething();
// After calling doSomething(), we are guaranteed that blah blah (associated with state after code executes)

Enforcement In general, difficult, because it is hard to programatically determine what specific line of code a comment is related to, if any.

@neithernut
Copy link

Also, while the CppCoreGuidelines examples generally prefer in-line comments [...]

The nature of comments detailing on some specific code aspect in examples differs from "regular" comments you may find in actual production code. In the guidelines, for example, you may often find inline comments telling you whether a compiler is expected to accept a specific line or not, often including a reason in the latter case. So you cannot necessarily conclude what comments should look like in production code from these example snippets.

@hsutter
Copy link
Contributor

hsutter commented Jun 6, 2024

Editors call: Thanks! We don't dictate style, and leave those issues to the NL section which is about "if you don't have rules, you can consider using these."

@hsutter hsutter closed this as completed Jun 6, 2024
@hsutter hsutter self-assigned this Jun 6, 2024
@pmolodo
Copy link
Author

pmolodo commented Jun 25, 2024

Editors call: Thanks! We don't dictate style, and leave those issues to the NL section which is about "if you don't have rules, you can consider using these."

I'm a little confused - this WAS a submission for the NL section...?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants