- 1. Motivation
- 2. Language standard
- 3. Formatting
- 4. Statements
- 5. Declarations
- 6. Scoping
- 7. Headers
- 8. Naming
- 9. Comments
- 10. Logging
- 11. Classes
- 12. Other conventions
When working in a large group, the two most important values are readability and maintainability. We code for other people, not computers. To accomplish these goals, we have created a unified set of code conventions.
In the repository root directory, there is a .clang-format file that implements the rules as specified here. You are encouraged to run clang-format on any newly created files. It is currently not recommended to do so on preexisting files because all the formatting changes will clutter your commits and pull request.
When you create a pull request, the PR build job will run clang-format on your commits and provide patches for any parts that don't satisfy the current .clang-format rules. You should apply these patches and amend your pull request accordingly.
Conventions can be bent or broken in the interest of making code more readable and maintainable. However, if you submit a patch that contains excessive style conflicts, you may be asked to improve your code before your pull request is reviewed.
We currently target the C++17 language standard. Do use C++17 features when possible (and supported by all target platforms). Do not use C++20 features.
The ColumnLimit in .clang-format is set to 100 which defines line length (in general where lines should be broken) that allows two editors side by side on a 1080p screen for diffs.
Curly braces always go on a new line.
for (int i = 0; i < t; i++)
{
[...]
}
if (true)
{
[...]
}
class Dummy
{
[...]
};Use spaces as tab policy with an indentation size of 2. Opening curly braces increase the level of indentation. Closing curly braces decrease the level of indentation.
Exception: Do not indent namespaces to simplify nesting them and wrapping .cpp files in a namespace.
namespace KODI
{
namespace UTILS
{
class ILogger
{
public:
virtual void Log(...) = 0;
virtual ~ILogger() {}
}
}
}Insert a new line before every:
- else in an if statement
- catch in a try statement
Put the consequent on a new line if not in curly braces anyway. Keep else if statements on one line. Do not put a condition and a following statement on a single line.
✅ Good:
if (true)
return;
if (true)
{
[...]
}
else if (false)
{
return;
}
else
return;❌ Bad:
if (true) return;switch (cmd)
{
case x:
{
doSomething();
break;
}
case x:
case z:
return true;
default:
doSomething();
}try
{
[...]
}
catch (std::exception& e)
{
[...]
throw;
}
catch (...)
{
[...]
}Conventional operators have to be surrounded by one space on each side.
a = (b + c) * d;Control statement keywords have to be separated from opening parentheses by one space.
while (true);
for (int i = 0; i < x; i++);When conditions are used without parentheses, it is preferable to add a new line, to make the next block of code more readable.
if (true)
return;
if (true)Commas have to be followed by one space.
void Dummy::Method(int a, int b, int c);Initializer lists have one space after each element (including comma), but no surrounding spaces.
constexpr int aNiceArray[] = {1, 2, 3};Do not use whitespace to vertically align around operators or values. This causes problems on code review if one needs to realign all values to their new position, producing unnecessarily large diffs.
✅ Good:
int value1{1};
int value2{2};
CExampleClass* exampleClass{};
CBiggerExampleClass* biggerExampleClass{};
exampleClass = new CExampleClass(value1, value2);
biggerExampleClass = new CBiggerExampleClass(value1, value2);
exampleClass->InitExample();
biggerExampleClass->InitExample();❌ Bad:
int value1 {1};
int value2 {2};
[...]
CExampleClass *exampleClass {};
CBiggerExampleClass *biggerExampleClass {};
[...]
exampleClass = new CExampleClass (value1, value2);
biggerExampleClass = new CBiggerExampleClass(value1, value2);
[...]
exampleClass ->InitExample();
biggerExampleClass->InitExample();Do not write void in empty function parameter declarations.
✅ Good:
void Test();❌ Bad:
void Test(void);There are some special situations where vertical alignment and longer lines does greatly aid readability, for example the initialization of some table-like multiple row structures. In these rare cases exceptions can be made to the formatting rules on vertical alignment, and the defined line length can be exceeded.
To prevent the layout from being reformatted, tell clang-format to disable formatting on that section of code by surrounding it with the special comments // clang-format off and // clang-format on.
For example:
// clang-format off
static const CGUIDialogMediaFilter::Filter filterList[] = {
{ "movies", FieldTitle, 556, SettingType::String, "edit", "string", CDatabaseQueryRule::OPERATOR_CONTAINS },
{ "movies", FieldRating, 563, SettingType::Number, "range", "number", CDatabaseQueryRule::OPERATOR_BETWEEN },
{ "movies", FieldUserRating, 38018, SettingType::Integer, "range", "integer", CDatabaseQueryRule::OPERATOR_BETWEEN },
...
{ "songs", FieldSource, 39030, SettingType::List, "list", "string", CDatabaseQueryRule::OPERATOR_EQUALS },
};
// clang-format on
The other code guidelines will still need to be applied within the delimited lines of code, but with clang-format off care will be needed to check these manually. Using vertical alignment means that sometimes the entire block of code may need to be realigned, good judgement should be used in each case with the objective of preserving readability yet minimising impact.
This is to be used with discretion, marking large amounts of code to be left unformatted by clang-format without reasonable justification will be rejected.
Do not put multiple statements on a single line. Always use a new line for a new statement. It is much easier to debug if one can pinpoint a precise line number.
✅ Good:
std::vector<std::string> test;
test.push_back("foobar");❌ Bad:
std::vector<std::string> test; test.push_back("foobar");In every switch structure, always include a default case, unless switching on an enum and all enum values are explicitly handled.
Always declare a variable close to its use and not before a block of code that does not use it.
✅ Good:
int x{3};
CLog::Log("test: {}", x); // variable used just after its declaration❌ Bad:
int x{3}; // variable not immediately used by the next block of code
[...many lines of code that do not use variable x...]
CLog::Log("test: {}", x);Do not put multiple declarations on a single line. This avoids confusion with differing pointers, references, and initialization values on the same line (cf. ISO C++ guidelines).
✅ Good:
char* a;
char b;❌ Bad:
char* a, b;Left-align * and & to the base type they modify.
✅ Good:
char* a;
void test(const std::string& b);❌ Bad:
char *a;
char * b;
void test(const std::string &b);(This is adopted from the HP C++ Coding Guidelines: "The characters * and & are to be written with the type of variables instead of with the name of variables in order to emphasize that they are part of the type definition.")
Place const and similar modifiers in front of the type they modify.
✅ Good:
void Test(const std::string& a);
const int* const someIntPointer;❌ Bad:
void Test(std::string const& a);
int const * const someIntPointer;Make sure that variables are initialized appropriately at declaration or soon afterwards. This is especially important for fundamental type variables that do not have any constructor. Zero-initialize with {}.
✅ Good:
int x{3};
int* y{nullptr};
bool z = false;
std::string text; // not primitive
KindOfStruct theStruct{}; // POD structures or structures with uninitalised members must be initialised with empty brackets
Log::Log("test: {}, {}, {}", x, y, z);❌ Bad:
int x; // used uninitialized
int* y = nullptr; // default-initialization not using {}
bool z{}; // no value explicitly declared for fundamental type, preferable for better reading
std::string text{}; // has its default constructor
Log::Log("test: {}, {}, {}", x, y, z);We allow variable initialization using any of the C++ forms {}, = or ().
However, we would like to point out some optional suggestions to follow:
- Prefer the
{}form to others, because this permits explicit type checking to avoid unwanted narrowing conversions. - Prefer the
{}form when initializing a class/struct variable. - Specify an explicit initialization value, at least for fundamental types.
Try to put all code into appropriate namespaces (e.g. following directory structure) and avoid polluting the global namespace.
Put functions local to a compilation unit into an anonymous namespace.
✅ Good:
namespace
{
void test();
} // unnamed namespace❌ Bad:
static void test();Included header files have to be sorted (case sensitive) alphabetically to prevent duplicates and allow better overview, with an empty line clearly separating sections.
Header order has to be:
- Own header file
- Other Kodi includes (platform independent)
- Other Kodi includes (platform specific)
- C and C++ system files
- Other libraries' header files
- special Kodi headers (PlatformDefs.h, system.h and system_gl.h)
#include "PVRManager.h"
#include "Application.h"
#include "ServiceBroker.h"
#include "addons/AddonInstaller.h"
#include "dialogs/GUIDialogExtendedProgressBar.h"
#include "messaging/ApplicationMessenger.h"
#include "messaging/ThreadMessage.h"
#include "messaging/helpers/DialogHelper.h"
#include "music/MusicDatabase.h"
#include "music/tags/MusicInfoTag.h"
#include "network/Network.h"
#include "pvr/addons/PVRClients.h"
#include "pvr/channels/PVRChannel.h"
#include "settings/Settings.h"
#include "threads/SingleLock.h"
#include "utils/JobManager.h"
#include "utils/Variant.h"
#include "utils/log.h"
#include "video/VideoDatabase.h"
#include <cassert>
#include <utility>
#include <libavutil/pixfmt.h>If the headers aren't sorted, either do your best to match the existing order, or precede your commit with an alphabetization commit.
If possible, avoid including headers in another header. Instead, you can forward-declare the class and use a std::unique_ptr (or similar):
class CFileItem;
class Example
{
...
std::unique_ptr<CFileItem> m_fileItem;
}To use C symbols use C++ wrappers headers, by using the std namespace prefix.
✅ Good:
#include <cstring>
[...]
size_t n = std::strlen(str);❌ Bad:
#include <string.h> // C header
[...]
size_t n = strlen(str); // missing std namespaceUse upper case with underscores.
namespace KODI
{
[...]
} // namespace KODIUse upper case with underscores.
constexpr int MY_CONSTANT = 1;Use PascalCase for the enum name and upper case with underscores for the values.
enum class Dummy
{
VALUE_X,
VALUE_Y
};Use PascalCase and prefix with an uppercase I. Filename has to match the interface name without the prefixed I, e.g. Logger.h
class ILogger
{
public:
virtual void Log(...) = 0;
virtual ~ILogger() {}
}Use PascalCase and prefix with an uppercase C. Filename has to match the class name without the prefixed C, e.g. Logger.cpp
class CLogger : public ILogger
{
public:
void Log(...) override;
}Use PascalCase.
struct InfoChar
{
bool m_isBold{false};
}Use PascalCase always, uppercasing the first letter even if the methods are private or protected. Method parameters start with lower case and follow CamelCase style, without type prefixing (Systems Hungarian notation).
void MyDummyClass::DoSomething(int limitBound);Variables start with lower case and follow CamelCase style. Type prefixing (Systems Hungarian notation) is discouraged.
✅ Good:
bool isSunYellow{true};❌ Bad:
bool bSunYellow{true}; // type prefixingPrefix non-static member variables with m_. Prefix static member variables with ms_.
int m_variableA;
static int ms_variableB;Prefix global variables with g_
int g_globalVariableA;WARNING: Avoid use of globals as far as reasonably possible. We generally do not want to introduce new global variables.
Use // for inline single-line and multi-line comments. Use /* */ for the copyright comment at the beginning of the file. SPDX license headers are required for all code files (see example below).
✅ Good:
/*
* Copyright (C) 2005-2018 Team Kodi
* This file is part of Kodi - https://kodi.tv
*
* SPDX-License-Identifier: GPL-2.0-or-later
* See LICENSES/README.md for more information.
*/
// Nice comment
// This can also continue for multiple lines:
// I am the second line.❌ Bad:
/* A comment */
//another commentNew classes and functions are expected to have Doxygen comments describing their purpose, parameters, and behavior in the header file. However, do not describe trivialities - it only adds visual noise. Use the Qt style with exclamation mark (/*! */) and backslash for doxygen commands (e.g. \brief).
✅ Good:
/*!
* \brief Splits the given input string using the given delimiter into separate strings.
*
* If the given input string is empty, nothing will be put into the target iterator.
*
* \param destination the beginning of the destination range
* \param input input string to be split
* \param delimiter delimiter to be used to split the input string
* \param maxStrings (optional) maximum number of splitted strings
* \return output iterator to the element in the destination range one past the last element
* that was put there
*/
template<typename OutputIt>
static OutputIt SplitTo(OutputIt destination, const std::string& input, const std::string& delimiter, unsigned int maxStrings = 0);❌ Bad:
/**
* @brief Function for documentation purposes (javadoc style)
*/
void TestFunction();
void ReallyComplicatedFunction(); // does something really complicated
/*!
* \brief Frobnicate a parameter
* \param param parameter to frobnicate
* \result frobnication result
*/
int Frobnicate(int param);Use the provided logging function CLog::Log. Do not log to standard output or standard error using e.g. printf or std::cout.
The Log function uses the fmt library for formatting log messages. Basically, you can use {} as placeholder for anything and list the parameters to insert after the message similarly to printf. See here for the detailed syntax and below for an example.
✅ Good:
CLog::Log(LOGDEBUG, "Window size: {}x{}", width, height);
CLog::LogF(LOGERROR, "An error occurred in window \"{}\"", name); // Use helper function `CLog::LogF` to print also the name of method.❌ Bad:
CLog::Log(LOGWARNING, "Window size: %dx%d", width, height); // printf-style format strings are possible, but discouraged; also the message does not warrant the warning level
CLog::Log(LOGERROR, "{}: An error occurred in window \"{}\"", __FUNCTION__, name); // Do not use __FUNCTION__ macro, use `CLog::LogF` instead.
printf("Window size: %dx%d", width, height);
std::cout << "Window size: " << width << "x" << height << std::endl;The predefined logging levels are DEBUG, INFO, WARNING, ERROR, and FATAL. Use anything INFO and above sparingly since it will be written to the log by default. Too many messages will clutter the log and reduce visibility of important information. DEBUG messages are only written when debug logging is enabled.
Make class data members private. Think twice before using protected for data members and functions, as its level of encapsulation is effectively equivalent to public.
Try to mark member functions of classes as const whenever reasonable.
When overriding virtual functions of a base class, add the override keyword. Do not add the virtual keyword.
✅ Good:
class CLogger : public ILogger
{
public:
void Log(...) override;
}❌ Bad:
class CLogger : public ILogger
{
public:
virtual void Log(...) override;
}Use default member initialization instead of initializer lists or constructor assignments whenever it makes sense.
class Foo
{
bool m_fooBar{false};
};A class with any virtual functions should have a destructor that is either public and virtual or else protected and non-virtual (cf. ISO C++ guidelines).
For lines up to line length everything stays on one line, excluding the braces which must be on the following lines.
MyClass::CMyClass(bool argOne, int argTwo) : m_argOne(argOne), m_argTwo(argTwo)
{
}For longer lines, insert a line break before the colon and/or after the comma.
MyClass::CMyClass(bool argOne,
int argTwo,
const std::string& textArg,
const std::shared_ptr<CMyOtherClass>& myOtherClass)
: m_argOne(argOne),
m_argTwo(argTwo),
m_textArg(textArg),
m_myOtherClass(myOtherClass)
{
}For functions that have multiple output values, prefer using a struct or tuple return value over output parameters that use pointers or references (cf. ISO C++ guidelines). In general, try to avoid output parameters completely (cf. ISO C++ guidelines, Google C++ Style Guide). At the function call site, it is completely invisible that actually a reference is being passed and the value might be modified. Return semantics make it clear what is happening.
New code has to use C++ style casts and not older C style casts. When modifying existing code the developer can choose to update it to C++ style casts or leave as is. Whenever a dynamic_cast is used to cast to a pointer type, the result can be nullptr and needs to be checked accordingly.
✅ Good:
char m_dataChar{...};
uint8_t m_dataInt = static_cast<uint8_t>(m_dataChar);❌ Bad:
char m_dataChar{...};
uint8_t m_dataInt = (uint8_t)m_dataChar;Prefer the use of nullptr instead of NULL. nullptr is a typesafe version and as such can't be implicitly converted to int or anything else.
Feel free to use auto wherever it improves readability, without abusing it when it is not the case.
- Good places are iterators or types that have multiple sub-levels in a namespace.
- Bad places are code that expects a certain type that is not immediately clear from the context, or when you declare fundamental types.
✅ Good:
[...]
constexpr float START_POINT = 5;
[...]
auto i = var.begin();
std::vector<CSizeInt> list;
for (const auto j : list)
{
[...]
}❌ Bad:
[...]
constexpr auto START_POINT = 5; // may cause problems due to wrong type deduced, many auto variables make reading difficult
[...]
std::map<std::string, std::vector<int>>::iterator i = var.begin();Use range-based for loops wherever it makes sense. If iterators are used, see above about using auto.
for (const auto& : var)
{
[...]
}Remove const if the value has to be modified. Do not use references to fundamental types that are not modified.
Use #pragma once.
✅ Good:
#pragma once❌ Bad:
#ifndef SOME_FILE_H_INCLUDED
#define SOME_FILE_H_INCLUDED
[...]
#endifUse the C++ using syntax when aliasing types (encouraged when it improves readability).
✅ Good:
using SizeType = int;❌ Bad:
typedef int SizeType;Usage of goto is discouraged.
Try to avoid using C macros. In many cases, they can be easily substituted with other non-macro constructs.
Prefer constexpr over const for constants when possible. Try to mark functions constexpr when reasonable.
Prefer std::string_view over std::string when reasonable. Good examples are constants or method arguments. In the latter case, it is not required to declare the argument as reference or const, since the data source of string views are immutable by definition. A bad example is when you need a NUL-terminated C string, e.g. to interact with a C API. std::string_view does not offer a c_str() function like std::string does, but if you do not need a C string you can use data() to get the raw source of the data.
Main reasons why we prefer std::string_view are: execution performance, no memory allocations, substrings can be made without copy, and the possibility to reuse the same data without reallocation.
Avoid using std::string_view when you are not sure where the source of data is allocated, or as return value of a method. If not handled properly, the source (storage) of the data may go out of scope. As a consequence, the program enters undefined behavior and may crash, behave strangely, or introduce potential security issues.
✅ Good:
namespace
{
constexpr std::string_view CONSTANT_FOO{"foo-bar"};
} // unnamed namespace
void CClass::SetText(std::string_view value)
{
CLog::LogF(LOGDEBUG, "My name is {}", value);
}
[...]
SetText(CONSTANT_FOO.substr(0, 3)); // substr returns a modified view of the same string_view, thus without allocations❌ Bad:
namespace
{
constexpr std::string CONSTANT_FOO{"foo-bar"}; // using string_view will avoid a memory allocation
} // unnamed namespace
void CClass::SetText(const std::string& value) // despite being declared as a reference, using string_view will result in a lower overhead in many cases (e.g., when passing a C string literal)
{
CLog::LogF(LOGDEBUG, "My name is {}", value);
}
[...]
SetText(CONSTANT_FOO.substr(0, 3)); // using string_view will avoid a memory allocation due to substr