Core Guidelines are not Rules

There is a difference between guidelines and rules. Boiling down guidelines to one-sentence rules has drawbacks that make your code harder to understand.

The famous quote by Captain Barbossa from _Pirates of the Caribbean_ says “The Code is more what you’d call guidelines than actual rules.” This implies that there is a difference between the two.

Rules are too convenient

Of course, being a pirate, Barbossa finds it convenient that The Code is just guidelines. It means he can throw them over board whenever they don’t suit him. For developers, strict rules seem more convenient. The compiler is very strict when it comes to what it accepts. And we have so much complicated problems to solve that a set of rules to follow blindly takes a lot off our shoulders.

While we are at it, we like to boil those rules down to simple sentences. When we look at guidelines and best practices, they usually have more than one sentence – they state reasons, and often enough also exceptions.

Boiling that kind of guideline down to a simple rule and following it no matter what can make our code worse. We’d be adding boilerplate or workarounds just to not have to think about and justify why we safely can ignore a specific guideline in a certain case.

An example

Let us look at a set of C++ Core Guidelines, and how they interact with each other.

C.20: If you can avoid defining default operations, do

This is the “Rule of Zero”. Well, oops. This guideline actually is called a rule, and it has every right to be.

It can be seen as a rule because there’s not much more to it than the title says, the reason is clearer semantics.

C.21: If you define or =delete any copy, move, or destructor function, define or =delete them all

This is the “Rule of five”. A rule, again. (Bear with me, I am getting to the point I want to make about guidelines shortly).

This is the other side of the coin to the Rule of Zero: If you have to deal with any of these functions, you are working on non-trivial resource or responsibility mangement. The Single Responsibility Principle implies that this should be the only thing your class does.

This rule or guideline is somewhat disputed. After all, there is a set of language rules that defines when the compiler will or will not generate some of these member functions, when others are explicitly defined. So, in theory, we would not always have to define or delete them all but rely on the compiler to do the right thing for us.

However, the Core Guidelines state as rationale for this slightly stricter-than-necessary rule, that by explicitly expressing the intent, we avoid unwanted effects like involuntary turning off move operations.

Also, I’d argue thath the simplicity of this rule makes code more readable. While we may have to write up to four more lines than strictly necessary, the code shows our intent. With all five special member mentioned, readers don’t have to memorize the exact rules the compiler follows for defaulting or not declaring them.

There is a corollary to these two rules: Complex classes that would have to define one of those operations should instead be composed of smaller classes that implement them. For example, instead of using multiple raw pointer data members and managing all that memory in your class, use smart pointer members instead which take care of the memory management.

C.12: Don’t make data members const or references in a copyable or movable type

In most instances where I have seen this taken over into coding conventions, this is written as “Don’t make data members const or references”. There also is a clang-tidy check that will give you a warning: member 'm' of type 'T &' is a reference [cppcoreguidelines-avoid-const-or-ref-data-members]. While it does give that warning only on copyable/movable types, it does not state so.

What this guideline is about is value semantics: When you have a class of value objects, you often want to be able to assign different values to them. const and reference data members make values of a class non-assignable, while they still can be copy-constructed and move-constructed. So, in that context, this guideline is right and makes a lot of sense.

The exception to the rule guideline

Besides value objects, in many applications we also have a lot of what I would call service objects. They get created and then sit there, doing their job, but they don’t represent a value that needs to get passed around. Often, these service objects need to talk to other service objects. In many applications, a net of service objects is build that hold references to other service objects and pass around values to perform the job of the application.

The natural way to implement these service objects is to have them hold references to their dependencies. Quite often, they incidentally are copyable or movable even though they don’t need to be, which will trigger the mentioned warning and/or comments in the code review that this is “against the rules”.

In surroundings where guidelines are interpreted as rules that have to be followed, the reaction to this may play out as follows:

1. Read up on the metioned Core Guideline and follow the note that says “use a pointer instead”.
2. Get complaints by static analysis that now there has to be a null pointer check whenever this is accessed.
3. Try to get the team to add the GSL support library to the project, which is a rather daunting task in large corporate environments.
4. Fall back to explicitly making the object non-assignable to silence the warning and colleagues.
5. Add more boilerplate to satisfy the Rule of Five, but ignore that this violates the Rule of Zero.

Conclusion

Guidelines may contradict each other when taken out of context. Instead of taking them as rules to follow blindly, we should remind ourselves that they are “more what you’d call guidelines than actual rules”. Think about their rationale and whether they really apply to your situation.

// Service class does not need to be assignable
// NOLINTBEGIN(*-ref-data-members)

class MyApplicationService {
    ThatOtherService& myDependency; 
/* ... */
};

// NOLINTEND(*-ref-data-members)