C++ coding conventions#
The following is not meant to be a complete description of our coding conventions. When in doubt, follow the example of the existing code.
See also our style conventions.
The search code is written in C++20. Refer to requirements for the C++ compiler and Python versions we currently use to see what language features you may use.
General recommendation#
We generally follow the recommendations in the book C++ Coding Standards: 101 Rules, Guidelines, and Best Practices by Herb Sutter and Andrei Alexandrescu.
Files, Subdirectories, namespaces and CMake libraries#
Files:
- File names should be in snake_case and end in
.hor.cc. For example:- Code for the merge-and-shrink heuristic lives in
merge_and_shrink_heuristic.handmerge_and_shrink_heuristic.cc. - Code for the Pareto open list lives in
pareto_open_list.handpareto_open_list.cc.
- Code for the merge-and-shrink heuristic lives in
Subdirectories:
- We don't use nested subdirectories (at least for now).
- There are two types of subdirectories:
- component subdirectories correspond to a single component (e.g. one subdirectory for the PDB code, one for the landmarks code, one for the merge-and-shrink code).
- grouping subdirectories group together many components that are related and too small to deserve their individual subdirectories (e.g. one for all open list variants and one for all heuristics that don't need their own subdirectory).
- Subdirectory names should be kept short and in snake_case.
Namespaces:
- We don't use nested namespaces (at least for now).
- Every component (= CMake library) should correspond to a namespace.
- For components that have their own directory, the namespace name is the same as the directory name.
- For components that are too small for their own directory, the namespace name is the same as the main file name without the suffix.
- We don't use
using namespacefor our own namespaces.
CMake libraries:
- There's a 1:1 correspondence between namespaces and CMake libraries.
Open questions:
- We might introduce a namespace for the core code, i.e., the code that is necessary to build the planner. Right now it is in the global namespace.
- We might eventually consider settling for shorter class names and/or
filenames in cases where the directory name or namespace already
provide the necessary context. For example, we might say
AlternationOpenList::OpenListorAlternation::OpenListinstead ofAlternationOpenList. (If you want to argue for this, this would need further discussion.)
Header file guards#
Macro names for header file guards follow this algorithm:
- Take the filename, including subdirectory name if in a subdirectory.
- Convert to uppercase.
- Replace all
.and/with_.
Example: learning/state_space_sample.h becomes
LEARNING_STATE_SPACE_SAMPLE_H.
Guard blocks should look like this:
That's all. In particular, don't add comments to the preprocessor directives and don't add further underscores.
Constructors, destructors and assignment operators#
- Add default destructor only for base classes (i.e., if other classes derive from them). The destructor should of course be virtual.
- Explicitly remove copy constructor, i.e., declare it as
= delete, for most types, especially those created by plug-ins and used polymorphically. Generally, we want to permit copy constructions only for cases where we've explicitly decided that it's useful. Many of our objects are heavy-weight and should not be copied.
Function signatures#
- Use
constmethods whenever appropriate. - Pass
strings byconstreference. - When overriding a virtual method, mention
virtualagain in the declaration and mark it asoverride(i.e.,virtual int foo() override;rather thanint foo();). - Mark function declarations in headers as
extern(global functions). - Mark function declarations in .cc files
static(local functions).
const attributes#
We generally don't make attributes of classes const. See the
attached discussion
for the rationale behind this convention. (This is a convention that may
be changed if there is sufficient support, but while it is the way it
is, we should all follow the same style.) Exceptions include:
- static constants like
static const int UNKNOWN = -1;and enumeration values (all cases where we would currently useALL_CAPS) - pointers and references to const (not really an exception
because we do not mark the pointers as const, i.e., we would
write
const Frobnicator *frobnicator;, but notconst Frobnicator *frobnicator const;. - attributes referring to plug-in parameters passed in through the option parser (This may seem a somewhat arbitrary convention, and perhaps it is, but for good or bad it does describe the one exception to the general rules that currently occurs commonly in the code.)
The same rules apply to function parameters and local variables.
push_back vs. emplace_back#
-
For a
vector<T>, usepush_backwith arguments of typeTorT&&to copy or move an already constructed object into the container, and useemplace_backin other cases, where the object can be directly constructed inside the container.
Examples:
Foo x = ...; // construct a Foo
vector<Foo> foos;
foos.push_back(move(x)); // move the constructed Foo into the container
foos.emplace_back(44, 22); // Foo has a constructor that takes two ints. Use emplace_back to directly construct the object inside the container.
Anti-idioms#
- Don't write
NULLor0for null pointers. Usenullptr. - Don't write
(ptr != nullptr). Write(ptr). - Don't write
(ptr == nullptr). Write(!ptr). - Don't write
(seq.size() == 0). Write(seq.empty()). - Don't write
(seq.size() != 0). Write(!seq.empty()). - Don't append underscores to constructor variables. Use the same name as the member variable (preferrable) or a different name.
Passing and storing tasks#
- By default, pass
const TaskProxy &. -
Pass
const shared_ptr<AbstractTask> &only in the following situations:- the callee should participate in the ownership of the task
- the callee creates a delegating task based on the given task (even if it's only a temporary)
Conceptually, it's less clear that this is desirable, but with our current design you cannot create a delegating task without (co-) owning the task.
- If the callee only needs access to a certain aspect of the task, it
is preferable to make this explicit and only pass e.g.
const OperatorsProxy &orconst [[VariablesProxy]] &. - If the task is needed after an object's construction, store a
TaskProxyas a member variable. - Avoid storing large collections of proxies that carry redundant
information. For example, a vector of 10000
FactProxyinstances (of the same task) contains 10000 copies of the same abstract task pointer. - Avoid using proxies in performance-critical code.
Exceptions#
Derive custom exception classes from utils::Exception.
- This rule does not apply to internal exceptions never seen by the
user, such as
HillClimbingTimeoutin the PDB code. - The
printmethod should write tocerr. See the existing examples ofutils::Exceptionsubclasses. utils::Exceptionis intentionally not derived fromstd::exception. See the attached file for some discussion.
Miscellaneous#
- Prefer
template<typename T>overtemplate<class T>.