Coding Guidelines

This page contains a set of guidelines regarding the preferred coding style for new code. Old code follows different standards and should not be modified for the sole purpose of making it follow the guidelines.

In a nutshell, new or refactored Freeciv21 code is written in modern C++ (the standard version evolves depending on what the targeted compilers support). This coding style strives to achieve a balance between old code originally written in C and the new possibilities brought by C++, so that new and old code do not look too different. At this time we are targeting C++17.

The rules listed on this page are guidelines. They represent the preferred way of writing code for Freeciv21, but good code that does not follow the guidelines will not be rejected directly. However, you should expect suggestions for changes.

General

Code Formatting Rules

Freeciv21 uses an automated code formatting tool, clang-format version 11, to handle the most tedious parts of code formatting. All code must be formatted with clang-format. This relieves the admins from checking that you used the correct amount of whitespace after a comma, and you from having to fix it manually. The downside is that the tool is really picky and will sometimes want to reformat good-looking code. Even the maintainers are sometimes surprised! You will find more information about clang-format and detailed instructions in How to Submit a Pull Request.

Formatting the code as dictated by clang-format is mandatory.

Header Guards

Use #pragma once to protect headers against multiple inclusion:

#pragma once

This is equivalent to the ancient #ifndef/#define/#endif idiom, but much less error-prone. The syntax is supported by all mainstream compilers.

Includes

There are a collection of coding guidelines as it relates to includes:

  • It is considered a best practice to follow Include What You Use (IWYU) policies when deciding what to include.

  • Included headers should follow a pattern of most specific to least specific. A common pattern is:

    • self — This is how we name the .h header that corresponds with the same .cpp. You cannot get more specific.

    • generated – There are a few symbol headers that are generated at configure and should be included near the top.

    • utility — This directory has a large set of common symbols and is considered specific.

    • common — A very large segment of the shared symbols are defined here. After utility, these are quite specific.

    • dependency – After utility and common, we want to include external dependencies.

    • ai — The server makes heavy usage of the game Artificial Intelligence (AI) so we include first.

    • server — The server provides an abstraction layer that we make a lot of use of, but its lower on the include hierarchy.

    • client — The client is the presentation layer.

    • Qt — We rely on the Qt system. These are highly abstracted from our base code and should be included near the last.

    • C Standard Library (std) — Any reliance on the std is even more abstracted than Qt and should be last.

Example:

// self
#include "foo.h"

// generated
#include <fc_config.h>

// utility
#include "fcintl.h"

// common
#include "city.h"
#include "tile.h"
#include "unit.h"

// client
#include "layer.h"

// Qt
#include <QString>
#include <QWidget>

// std
#include <map>
#include <vector>

Some helpful tips:

  1. The use of clang-tidy is encouraged.

freeciv21$ cmake --preset clang
freeciv21$ clang-tidy -p build-clang path/to/cpp-or-h/file

Start with the header (.h), resolve any complaints and then move to the .cpp. Multiple runs of clang-tidy can be useful.

  1. Do not include the generated specenum_gen.h file. The network protocol uses named enums heavily. The named enum will be in another header. Use code search to find if needed.

Forward Declarations

Whenever possible, use forward declarations in headers instead of adding an #include. This can greatly speed up compilation:

class QLabel;
class QLineEdit;

One can use a forward declaration if the class is only used to declare a pointer or reference variable, possibly as a function argument.

Documentation Comments

The function of all public entities (classes, functions, enumerations, enumerators, variables, …) should be described in a Doxygen-enabled comment. Classes and functions should use a multiline comment:

/**
 * Shows a message box greeting the user.
 */
void greet()
{
  QMessageBox::information(nullptr, _("Greetings"), _("Hello, user"));
}

These comments serve two purposes:

  • They help the reader understand the code.

  • They act as separators between functions in cpp files.

Single-line comments can be used for very simple methods whose implementation is included in a class definition, as well as for less complex constructs such as enumerations and variables. The use of Doxygen markup commands to provide more detailed descriptions is welcome, but in no way mandatory.

Two highly recommended Doxygen markup commands to include are \file and \class. When writing code, especially a new cpp file, including a comment with the \file markup near the top, but below the Copyright Notice gives the reader a better understanding of what the file’s purpose is. If the cpp file contains functions related to a class, then using the \class markup aids the reader in understanding what the class is doing.

Variable Declaration

Older Freeciv21 code will often have a block of variables defined all at once at the top of a function. The problem with this style is it makes it very easy to create variables that never get used or initialized.

When encountering this older style or writing new code, it is best to define the variable and give it an initial value right before it is used.

... some code
... some code

for (int i = 0; i < max_item; i++) {
  ... do something in the loop
}

Naming Convention

The developers have not agreed on a naming convention yet. In the meantime, most code has been following the former practice of using all_lowercase_letters in most cases. The only exception to this rule is for constant values (enumeration values and const static variables), for which UPPERCASE is generally used.

Private member variables should be prefixed with m_ and be placed at the bottom of the class:

class something
{
public:
  explicit something();
  virtual ~something();

private:
  int m_foo;
};

The freeciv Namespace

The freeciv namespace has been used to group classes created during refactoring efforts. This code is expected to follow higher standards than the rest of the code base, such as encapsulation and having minimal side effects.

The Anonymous Namespace

Symbols that are used in a single file, as support for other functions, should be defined in the anonymous namespace:

namespace /* anonymous */ {

const int IMPORTANT_CONSTANT = 5; ///< Very, very important

/**
 * Calculates a more important value of @c x.
 */
int some_internal_function(int x)
{
  return x + IMPORTANT_CONSTANT;
}

} // anonymous namespace

The compiler will generate arbitrary names for symbols in the anonymous namespace that will not clash with symbols defined elsewhere.

Premature Optimization

It is often useless to try and optimize a function before proving that it is inefficient by profiling the execution in an optimized build (Release or RelWithDebInfo). Most functions in Freeciv21 are not executed in tight loops. Prefer readable code over fast code.

C++ Features

C++ is a very complex language, but fortunately Freeciv21 only needs to use a relatively small subset. Qt, our main dependency, manages very well to minimize user exposure to confusing parts. If all you are doing is small changes here and there, you will most likely not need to know a lot about C++. As your projects grow in scale and complexity, you will likely want to learn more about the language. In addition to your preferred learning resources, it is useful to read guidelines written by C++ experts, for instance the C++ Core Guidelines edited by the very founder of C++.

We collect below a list of recommendations that we find useful in the context of Freeciv21.

Pass by Reference

When writing a function that takes a complex object (anything larger than a long long), use a constant reference:

QString foo(const QString &argument);
int bar(const std::vector<int> &argument);

Use const

Variables that are not modified should be declared const. While this is more of a personal preference for variables, it is especially important for functions taking references (see above).

Functions that do not modify their argument should make them const. Class methods that do not modify the object should also be marked const.

Use Encapsulation

Classes that are more complicated than C-like struct should not have any public variables. Getters and setters should be provided when needed.

Use auto

The auto keyword is useful to avoid typing the type of a variable, especially lengthy names used in the Standard Library. We recommend to use it whenever possible. Do not try to use auto for function arguments.

const auto &unit = tile->units.front();

Use STL Containers

Containers in the Standard Library should be preferred over Qt ones:

std::vector<unit *> foo;
std::map<int, int> bar;

One notable exception is QStringList, which should be preferred over other constructs because it integrates better with Qt.

This can have consequences on function signatures. For instance, a common C idiom is to pass an output array and a size as function parameter, whereas this is achieved in C++ by returning a std::vector (or, if performance is critical, passing a reference to a vector that the function fills).

Use <algorithm>

The C++ Standard Library provides a set of basic algorithms. Code using the standard algorithms is often more clear than hand-written loops, if only because experienced programmers will recognize the function name immediately.

Use Range-based for

Avoid using indices to iterate over containers. Prefer the much simpler range-based for:

for (const auto &city : player->cities) {
  // ...
}

Use Structured Bindings

Structured bindings are very useful when facing a std::pair, for instance when iterating over a map:

for (const auto &[key, value] : map) {

If you do not wish to use one of the variables, use _:

for (const auto &[key, _] : map) {
  // Use the key only

Use Smart Pointers

Instead of using new and delete, delegate the task to a smart pointer:

auto result = std::make_unique<cm_result>();

When facing a memory handling bug such as a double free, it is sometimes easier to rewrite the code using smart pointers than to understand the issue.

Smart pointers are rarely needed with Qt classes. The parent-child mechanism is the preferred way of handling ownership for classes deriving from QObject. In many other cases, Qt classes are meant to be used directly on the stack. This is valid for QString, QByteArray, QColor, QPixmap, and many others. If you are unsure, try to find an example in the Qt documentation.

Qt provides its own smart pointer for QObject, called QPointer. This pointer tracks the lifetime of the pointed-to object and is reset to nullptr if the object gets deleted. This is useful in some situations.

Todo

We would like to include some tips about the following topics in the future:

  • Logging

  • Qt Tips