Skip to content

Coding guidelines

Michał Majczak edited this page Aug 7, 2019 · 9 revisions

In PolyEngine, we wanted to establish a modern coding style that is easily readable, clean, maintainable, explicit and compliant with gaming industry conventions. Any suggestions for changes are welcome.

WARNING! These guidelines are currently work in progress.

1. General naming scheme

  1. All class names should be written in PascalCase, e.g. TransformComponent, GLRenderingDevice.
  2. All class fields names should be written in camelCase, e.g. movementSpeed, objectID. Additionally private and protected members should be marked with m_ prefix, e.g. m_myPrivateField.
  3. All local variables and constants should be written in camelCase, e.g. lastMovementSpeed, tmpCache.
  4. All functions/methods should be written in camelCase, e.g. update(), getSize().
  5. All global (or static, with exception to static method fields) constants should be written in UPPER_CASE, e.g. MEMORY_ALIGNMENT, MAX_COUNT.
  6. All global variables should be written in camelCase with "g_" prefix, e.g. g_engine, g_logger.
  7. All defines should be written in UPPER_CASE, e.g. MY_MACRO(...), MAX_FLOAT.
  8. When defining aliases using should be preferred to typedef, especially for function pointers.
  9. Use tabs to indent code instead of spaces.
  10. All engine code, if possible, should be placed under namespace pe. Each engine component should introduce his own nested namespace derived from the component name, e.g. pe::core, pe::engine, etc.
  11. Each namespace directly correlates to directories in the filesystem starting after Src directory.

2. Code layout

  1. All brackets should be placed in a new line of code. Exceptions are as follows:
  • Very short or empty method/functions/lambdas definitions, in such case these definitions should be placed in one line.
  • Constructor calls using brackets.

For example:

if (shouldUpdate)
{
    Update();
    shouldUpdate = false;
}
else
    shouldUpdate = true;
  1. Omitting brackets in if/while/for is allowed when there is only one line of code (statement).
  2. Breaking logical statements in if is allowed. In such case each sub-statement should be in separate line, with logical operator placed on it's left hand side. Please also remember to add brackets in such situation. e.g.
if (Foo()
    && Baz()
    && (Faz() || Gaz()))
{
  ...
}
  1. Indentation of each code block is mandatory.
  2. Always place space after if, while, for, and switch statements but never do it after function/method/class names e.g. if (shouldUpdate) ... and foo(...);.
  3. When using initialization list in class constructor : should be placed in new line and indented, followed by fields initialization. Bracket that signals start of constructor body should be placed in next line, unless body of the constructor is empty, e.g.
Foo::Foo(int a, int b)
  : A(a), B(b)
{
  Init();
}

Baz::Baz(int v)
  : Val(v) {}

Faz::Faz(int a, int b, int c)
  : A(a)
  , B(b)
  , C(c)
{
  Init();
}

3. Coding conventions

  1. Const correctness is very important. All methods that don't mutate state of the object must be marked as const or, preferably, constexpr (if possible). All function/method/class/global fields that can be marked as const or, preferably, constexpr should be made so.
  2. Usage of mutable is allowed in certain circumstances, mainly for caching purposes.
  3. Usage of ternary conditional operator is allowed when assigning or returning a value. Never use it for simple conditional code execution, i.e.
    Correct: float inv_x = (x != 0) ? 1.f/x : 0.f;
    Incorrect: isValid ? DoSth() : DoSthElse();
  4. Function/Method arguments that are custom types should be passed by const reference or r-value reference, depending on the intention. One exception from that rule is when implementation of such function must perform copy/move operation of that argument anyway, in such case it's much better (performance vise) to pass argument by value, but only when it implements move semantics.
    Normal examples: void Foo(int a, float b, const Vector& c);, void Baz(Dynarray<int>&& params);
    Exception example: (oversimplified): void Foo(Dynarray<int> a) { Dynarray<int> b = std::move(a); ... }
  5. If possible and applicable for a given class move semantics should be implemented.
  6. Relying on RVO is preferred over passing mutable references/pointers as function arguments, e.g.
    Correct: Dynarray<int> Dynarray<T>::FindAllIdx(const T& val) {...}
    Incorrect: void Dynarray<T>::FindAllIdx(const T& val, Dynarray<int>& result) {...}
  7. Using std::optional is preferred (in case of a lack of meaningful returned value) to using magic values, e.g. using value -1 when returning int to imply that sth was not found.
  8. Usage of this-> is discouraged and should only be used when necessary. Variable renaming is preferred in such case.
  9. Usage of exceptions is forbidden in performance sensitive parts of code. This rule is relaxed in areas like resource loading, networking, etc. or when using libraries that do not provide any other way of error handling. In such case separate exception types should be derived to describe the error.
  10. Includes using double quotes are prohibited. All includes should be done with angle brackets, e.g. #include <RTTI.hpp>
  11. All class fields that do not have default values should be initalized to default value in class declaration, e.g.
struct Foo
{
  int X = 0;
  int* Ptr = nullptr;
  std::unique_ptr<int> Uptr; // does not require initialization, it is initailized to nullptr by unique_ptr implementation.
};
  1. Forward declaration of structs, classes, enums, typedefs etc. in header files are much more preferred than additional includes.
  2. Usage of plain enums is prohibited. Instead, type safe enum class should be used. Using custom underlying types for enums is discouraged, but not prohibited.

4. Comments and documentation

  1. It's always better to write self documenting code, than to write hard to understand code and later explain it with comments. Nevertheless sometimes it's impossible, especially when working with foreign API.
  2. Comments should be used to explain every operation that is not easily understandable by reading the operation itself. 3. For single line comments // ... is preferred to /* ... */, which should be used for big blocks of comments. There should always be a space between // or /* and comment text itself, e.g. good: // Correct comment, e.g. bad: //Invalid comment.
  3. For documentation we use doxygen ( http://www.stack.nl/~dimitri/doxygen/ ). All doxygen comments should start with /// and all doxygen directives should be prefixed with @ instead of \, e.g.: /// @param value The value of aregument. instead of /** \param value The value of argument. */.
  4. All comments and documentation should be strictly meritorical. If a comment does not help to understand the code, or the reason for its usage then it's irrelevant and must be removed. There is no place for sarcasm, jokes and swearing in this project's code base.
  5. Todo's should be marked with // @todo in implementation and /// @todo in documentation. Same applies for fixme, note, remark, etc., e.g.: // @fixme Optimize this implementation. If possible a person responsible for such annotation should be mentioned.