Should we refer to the C++ Core Guidelines?

Question

Since last year's CppCon, there is an official set of C++ Guidelines. This long list offers rationales for many important coding style questions and is updated constantly.

Should we refer by links to this list when we do reviews of C++ code? If so, how much of the linked content should be in the answer?

Usually, a guideline consist of a number, a descriptive title, a reason, and some examples/counter examples. As seen in this example:

F.1: "Package" meaningful operations as carefully named functions

Reason

Factoring out common code makes code more readable, more likely to be reused, and limit errors from complex code. If something is a well-specified action, separate it out from its surrounding code and give it a name.

Example, don't

void read_and_print(istream& is)    // read and print an int
{
    int x;
    if (is >> x)
        cout << "the int is " << x << '\n';
    else
        cerr << "no int on input\n";
}

Almost everything is wrong with read_and_print. It reads, it writes (to a fixed ostream), it writes error messages (to a fixed ostream), it handles only ints. There is nothing to reuse, logically separate operations are intermingled and local variables are in scope after the end of their logical use. For a tiny example, this looks OK, but if the input operation, the output operation, and the error handling had been more complicated the tangled mess could become hard to understand.

Note

If you write a non-trivial lambda that potentially can be used in more than one place, give it a name by assigning it to a (usually non-local) variable.

Example

sort(a, b, [](T x, T y) { return x.rank() < y.rank() && x.value() < y.value(); });

Naming that lambda breaks up the expression into its logical parts and provides a strong hint to the meaning of the lambda.

auto lessT = [](T x, T y) { return x.rank() < y.rank() && x.value() < y.value(); };

sort(a, b, lessT);
find_if(a, b, lessT);

The shortest code is not always the best for performance or maintainability.

Exception: Loop bodies, including lambdas used as loop bodies, rarely need to be named. However, large loop bodies (e.g., dozens of lines or dozens of pages) can be a problem. The rule Keep functions short implies "Keep loop bodies short." Similarly, lambdas used as callback arguments are sometimes non-trivial, yet unlikely to be re-usable.

Enforcement

• See Keep functions short
• Flag identical and very similar lambdas used in different places.

I would propose to copy the number and title to our question and omit reasons and examples.

What do you think?

Example

Just to make my intention more clear, something like this could be in an answer:

In your code you have multiple locations where you calculate the GCD of two numbers. Instead of the ad hoc code, you should "Package" meaningful operations as carefully named functions (F.1):

int gcd(int a, int b) { return b > 0 ? gcd(b, a % b) : a; }

int a = 15, b = 27, gcdAB = gcd(a, b);
int c = 123, d = 42, gcdCD = gcd(c, d);

Or the guideline title can become a linked heading in the answer:

"Package" meaningful operations as carefully named functions (F.1)

In your code you have multiple locations where you calculate the GCD of two numbers. Instead of the ad hoc code, you should write and reuse functions:

int gcd(int a, int b) { return b > 0 ? gcd(b, a % b) : a; }

int a = 15, b = 27, gcdAB = gcd(a, b);
int c = 123, d = 42, gcdCD = gcd(c, d);

Similarly, you could write your own heading but link it to the appropriate guideline.

Answer 1

Should we refer by links to this list when we do reviews of C++ code?

It is up to the individual reviewer. If you consider this to be a valuable C++ resource, then absolutely do so. If others do not consider this a valuable resource, I see not reason for them to include it. If others are unaware, obviously they cannot include it. And even if others are aware and consider it a valuable resource, I don't think they should feel forced into including links to it left, right & center (or even referring to it at all).

If so, how much of the linked content should be in the answer?

Enough that your answer makes total & complete sense as if the guideline resource did not exist at all. This is no different than linking to any other external resource. I shouldn't have to follow a link to make sense of your answer.

I would propose to copy the number and title to our question and omit reasons and examples.

I would disagree. Or at least, I would disagree with one possible interpretation of what you've just said.

For example, the following would make for a terrible code review answer:

Your code is in violation of

F.1: "Package" meaningful operations as carefully named functions

This would be equivalent to me writing an answer that said:

You didn't write very readable code.

It's a link I've sprinkled throughout several of my answers, and while I'd argue that what the reader would find inside the link contains lots of valuable information, the link alone is not going to be enough to make a good answer.

Your answer should:

• point to specific parts of the asker's code
• explain what the relevant part of C++ guidelines mean
• explain why the asker's code is in conflict with the guideline
• provide examples specific to the asker's code of how to do it better