Coding guidelines

Code Quality and Style

Please use the clang-format (.clang-format file is provided in the root of the repository), if you want to contribute in order to format your code according to our coding guidelines.

If you are on a GNU/Linux system, you can use the format-files.sh shell script found in the tools folder

If you use Visual Studio it should use the .clang-format to automatically format the code

For the JSON files, you should use the Nodejs script found in the tools folder This scipt requires the latest LTS release of Nodejs and can be run with node format-json.js

Comments and Documentation

Cytopia uses Doxygen for generating and maintaining documentation of the codebase. Please ensure that you make liberal use of Doxygen-compatible "special comment blocks" in the code you write or modify.

At the very least, the following should all be properly commented:

• Classes

• Public Enums

• Public Members

These things should be commented with sufficient information to tell new contributors to what the item is used for and how to use it. Notes on things such as design patterns, known weaknesses or invalid inputs, "To Do" notes, and any additional information would be appreciated. Imagine the code is going to be handed off to someone slightly less competent than yourself with no knowledge of the project.

Currently, significant portions of the codebase are not properly documented, and launching a project to document everything would be laborious. When editing a file in a branch or fork that is not properly commented, please add appropriate comments to the existing code if possible.

Memory Management and Smart Pointers

You should only use pointers if you fall into at least one of these situations:

• You have an object that is larger than a pointer (assume 4 bytes) and you absolutely need to store it in multiple places.
• You need to control the construction and destruction of an object
• You are dealing with a large buffer of data (for example: pixel data, ASCII text)
• You are using a library API (like SDL2)

If you really do need a pointer, then first consider smart pointers. There are three types of smart pointers:

• Unique pointers (std::unique_ptr): Use them if there should only be one client accessing the object at a time. The client owns that object. It cannot copy the reference to that object. You can use std::move to move ownership of the object.
• Shared pointers (std::shared_ptr): Use them if you need more than one client owning the object at a time. If a std::shared_ptr object goes out of scope, its internal "usage counter" is decreased. When that counter reaches 0, memory is deallocated for you.
• Weak pointers (std::weak_ptr): Use them if you want to access an object without owning it. This prevents memory leaks of shared pointers that are common in Java. The overhead with weak pointers is that you must check before every access if the pointer is expired.

To create smart pointers, never use a constructor. Smart pointers have constructors which take a pointer to an existing object. The problem is that when you execute this code

std::shared_ptr<Data> pointer(new Data(10));

You allocate memory twice: once for Data, and once for the pointer object. If you use std::make_shared, then you only allocate memory once, and you also can specify a custom allocator instead of std::allocator. Another possible mistake would be this code

{
  Data* myData = new Data(10);
  std::shared_ptr ptr1(myData);
  std::shared_ptr ptr2(myData);
}

The problem is that ptr1 and ptr2 both think they have exclusive ownership of myData, so they both delete it and this creates a segmentation fault. With std::make_shared, this could never happen:

{
  std::shared_ptr ptr1 = std::make_shared<Data>(10);
  std::shared_ptr<Data> ptr2 = ptr1; // Reference count = 2
} // Data is only deleted once, when the reference count reaches 0

In general, never use the new and delete keywords. Use STL containers to deal with memory at all times. If you need to use APIs which deal with a dynamically-sized buffer, then use std::vector like this

std::vector<char> myBytes(numBytes);
fillMyBuffer(myBytes.data(), numBytes);
// Or
fillMyBuffer(&myBytes[0], numBytes);

As said earlier, STL containers allow us to specify a custom Allocator which can improve performance with memory pools.

Lastly, it is convenient to add aliases to pointer types. Those aliases should always follow the convention that

• For unique pointers, using TypeUPtr = std::unique_ptr<Type>;
• For shared pointers, using TypeSPtr = std::shared_ptr<Type>;
• For weak pointers, using TypeWPtr = std::weak_ptr<Type>;

Exceptions

When writing code, you should always think about exceptions. Use the noexcept specifier to guarantee the method will never throw. You have to prove that it will never throw, otherwise if an exception is thrown, then C++ will call std::terminate which will crash the application. If you know that the method could throw something, then document it with the @throws Doxygen command. As a reminder, any form of dynamic memory management has the chance to throw std::bad_alloc, so in general, constructors are never noexcept. If you are unsure if a method throws or not, then use noexcept. Finally, if you have to change some code, and remove a noexcept qualifier, then you have to verify that no noexcept method calls this method. If there are, then you will need to refactor them either by catching the exceptions or removing the noexcept specifier. You can see how refactoring can grow exponentially by removing noexcept. If it happens to be too much work, then you can keep the old method and mark is as deprecated using the @deprecated Doxygen command. Then create a new method with a different name, and the refactoring will become technical debt.

Assertions

Assertions are useful to verify assumptions on which the rest of the code (algorithm) depends on. The tested expression must be evaluated to true to pass assertion.

Static assertion

This type of assertions are compile-time checks and will make compilation error if the assumption does not hold on to the promise. It is the preferred type of assertion because will detect the issue early in compile-time and imposes no overhead at all.

static_assert(bool_constexpr, msg);

• bool_constexpr constant expression which returns bool
• msg optional message

Run-time assertion

This assertion will be checked during run-time and impose overhead when it is present, but in the release version, it is compiled out.

assert(condition)

• condition expresion which returns bool

In case that more code is needed than the simple expression to do assert testing, whole test block can be enveloped with NDEBUG macro and will be compiled out in release version.

#ifdef NDEBUG
    testcode;
    assert(condition);
#endif // NDEBUG