RogueScholar: Add toclimit-2 class to TOC div tag

2021-09-29T14:28:30Z

Add toclimit-2 class to TOC div tag

← Older revision		Revision as of 14:28, 29 September 2021
Line 2:		Line 2:

	== Motivation ==		== Motivation ==
	<div style="float: right; margin-left: ~~1.5em~~;">__TOC__</div>		<div class="toclimit-2" style="clear: both; margin-bottom: 0.5em; float: right; margin-left: 2em; width: auto;">__TOC__</div>
	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. 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 [[Merge window\|pull request]] is fully reviewed.		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. 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 [[Merge window\|pull request]] is fully reviewed.

RogueScholar: Add current coding conventions to eponymous article on wiki

2021-09-29T13:33:52Z

Add current coding conventions to eponymous article on wiki

New page

As a large and mature open source software project, Team Kodi has to integrate the work of hundreds of developers into a single, cohesive codebase. This is accomplished by insisting that those who wish to contribute do so according to a clear and explicit set of conventions, procedures and rubrics. This article is meant to organize them on a single page but is far from exhaustive; deeper study is almost certain to be required.

== Motivation ==
<div style="float: right; margin-left: 1.5em;">__TOC__</div>
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. 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 [[Merge window|pull request]] is fully reviewed.

In the repository root directory, there is a <code>.clang-format</code> file that implements the rules as specified here. You are encouraged to run <code>clang-format</code> 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.

== Language standard ==
We currently target the C++14 language standard, so do use C++14 features when possible, but do not use C++17 features.

== Formatting ==
=== Line length ===
The <code>ColumnLimit</code> in <code>.clang-format</code> is set to 100, which defines line length (i.e. where lines should be broken) that allows two documents to be displayed side-by-side on a 1080p screen for diffs at a standard DPI.

=== Braces ===
Curly braces always go on a new line.

<syntaxhighlight lang="c++">
for (int i = 0; i < t; i++)
{
[...]
}

if (true)
{
[...]
}

class Dummy
{
[...]
};
</syntaxhighlight>

=== Indentation ===
We use spaces as opposed to tabs to indent, with each level of indentation being two spaces further from the left margin than its parent. Opening curly braces increase the level of indentation for the code enclosed by them, and likewise closing curly braces decrease the level of indentation.

'''Exception:''' Do not indent namespaces to simplify nesting them and wrapping ''.cpp'' files in a namespace.

<syntaxhighlight lang="c++">
namespace KODI
{
namespace UTILS
{

class ILogger
{
public:
virtual void Log(...) = 0;
virtual ~ILogger() {}
}

}
}
</syntaxhighlight>

=== Control statements ===
Insert a new line before every:
* <code>else</code> in an <code>if</code> statement
* <code>catch</code> in a <code>try</code> statement
* <code>while</code> in a <code>do</code> statement

==== If/else ====
Put the consequent on a new line, if not in curly braces anyway. Keep <code>else if</code> (elif) statements on one line.

<syntaxhighlight lang="c++">
if (true)
return;

if (true)
{
[...]
}
else if (false)
{
return;
}
else
return;
</syntaxhighlight>

==== Switch cases ====
<syntaxhighlight lang="c++">
switch (cmd)
{
case x:
{
doSomething();
break;
}
case x:
case z:
return true;
default:
doSomething();
}
</syntaxhighlight>

==== Try/catch ====
<syntaxhighlight lang="c++">
try
{
[...]
}
catch (std::exception& e)
{
[...]
throw;
}
catch (...)
{
[...]
}
</syntaxhighlight>

=== Whitespace ===
Conventional operators have to be surrounded by one space on each side.
<syntaxhighlight lang="c++">
a = (b + c) * d;
</syntaxhighlight>

Control statement keywords have to be separated from opening parentheses by one space.
<syntaxhighlight lang="c++">
while (true);
for (int i = 0; i < x; i++);
</syntaxhighlight>

Commas have to be followed by one space.
<syntaxhighlight lang="c++">
void Dummy::Method(int a, int b, int c);
</syntaxhighlight>

Initializer lists have one space after each element (including the comma), but no surrounding spaces.
<syntaxhighlight lang="c++">
constexpr int aNiceArray[] = {1, 2, 3};
</syntaxhighlight>

=== Vertical alignment ===
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:
<syntaxhighlight lang="c++">
int value1{1};
int value2{2};
CExampleClass* exampleClass{};
CBiggerExampleClass* biggerExampleClass{};
exampleClass = new CExampleClass(value1, value2);
biggerExampleClass = new CBiggerExampleClass(value1, value2);
exampleClass->InitExample();
biggerExampleClass->InitExample();
</syntaxhighlight>

❌ Bad:
<syntaxhighlight lang="c++">
int value1 {1};
int value2 {2};
[...]
CExampleClass *exampleClass {};
CBiggerExampleClass *biggerExampleClass {};
[...]
exampleClass = new CExampleClass (value1, value2);
biggerExampleClass = new CBiggerExampleClass(value1, value2);
[...]
exampleClass ->InitExample();
biggerExampleClass->InitExample();
</syntaxhighlight>

=== Superfluous void ===
Do not write <code>void</code> in empty function parameter declarations.

✅ Good:
<syntaxhighlight lang="c++">
void Test();
</syntaxhighlight>

❌ Bad:
<syntaxhighlight lang="c++">
void Test(void);
</syntaxhighlight>

=== Exceptions to formatting rules ===
There are some special situations where vertical alignment and longer lines do 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 may be exceeded as well if it provides an obvious benefit.

The layout can be protected from being reformatted when <code>clang-format</code> is applied by adding <code>// clang-format off</code> and <code>// clang-format on</code> statements both above and below the lines of code which violate the rules. For example,
<syntaxhighlight lang="c++">
// 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
</syntaxhighlight>
but the other code guidelines will still need to be applied within the delimited lines of code, and with <code>clang-format</code> off, care will be needed to enforce these manually. Using vertical alignment means that sometimes the entire block of code may need to be realigned, and good judgement should be used in each case with the objective of preserving readability yet minimizing impact.

This is to be used with discretion; marking large amounts of code to be left unformatted by <code>clang-format</code> without reasonable justification will result in rejection of the submission.

== Statements ==
=== Multiple statements ===
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:
<syntaxhighlight lang="c++">
std::vector<std::string> test;
test.push_back("foobar");
</syntaxhighlight>

❌ Bad:
<syntaxhighlight lang="c++">
std::vector<std::string> test; test.push_back("foobar");
</syntaxhighlight>

=== Switch statement default case ===
In every <code>switch</code> structure, always include a <code>default</code> case, unless switching on an enum and all enum values are explicitly handled.

== Declarations ==
=== Multiple declarations ===
Do not put multiple declarations on a single line, this avoids confusion with differing pointers, references, and initialization values on the same line (cf. [https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es10-declare-one-name-only-per-declaration ISO C++ guidelines]).

✅ Good:
<syntaxhighlight lang="c++">
char* a;
char b;
</syntaxhighlight>

❌ Bad:
<syntaxhighlight lang="c++">
char* a, b;
</syntaxhighlight>

=== Pointer and reference types ===
Left-align <code>*</code> and <code>&</code> to the base type they modify.

✅ Good:
<syntaxhighlight lang="c++">
char* a;
void test(const std::string& b);
</syntaxhighlight>

❌ Bad:
<syntaxhighlight lang="c++">
char *a;
char * b;
void test(const std::string &b);
</syntaxhighlight>

This is adopted from the HP C++ Coding Guidelines: <blockquote style="font-size: 1.15em;">''The characters <code>*</code> and <code>&</code> 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.''</blockquote>

=== Const and other modifiers ===
Place <code>const</code> and similar modifiers in front of the type they modify.

✅ Good:
<syntaxhighlight lang="c++">
void Test(const std::string& a);
const int* const someIntPointer;
</syntaxhighlight>

❌ Bad:
<syntaxhighlight lang="c++">
void Test(std::string const& a);
int const * const someIntPointer;
</syntaxhighlight>

=== Initialization ===
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 <code>{}</code>.

✅ Good:
<syntaxhighlight lang="c++">
int x{};
int* y{};
CLog::Log("test: {} {}", x, y);
</syntaxhighlight>

❌ Bad:
<syntaxhighlight lang="c++">
int x; // used uninitialized
int* y = nullptr; // default-initialization not using {}
CLog::Log("test: {} {}", x, y);
</syntaxhighlight>

In general, prefer the <code>{}</code> initializer syntax over alternatives. This syntax is less ambiguous and disallows narrowing (cf. [https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-list ISO C++ guidelines]).

✅ Good:
<syntaxhighlight lang="c++">
int x{5};
int y{x};
</syntaxhighlight>

== Scoping ==
=== Namespaces ===
Try to put all code into appropriate namespaces (following the directory structure) and avoid polluting the global namespace.

=== Local functions ===
Put functions local to a compilation unit into an anonymous namespace.

✅ Good:
<syntaxhighlight lang="c++">
namespace
{

void test();

}
</syntaxhighlight>

❌ Bad:
<syntaxhighlight lang="c++">
static void test();
</syntaxhighlight>

== Headers ==
Included header files have to be sorted (case-sensitively) alphabetically to prevent duplicates and allow better overview, with an empty line clearly separating sections.

The header order has to be:
* Eponymous header file
* Platform-independent Kodi includes
* platform-specific Kodi includes
* C and C++ system files
* Other libraries' header files
* Special Kodi headers (i.e. PlatformDefs.h, system.h and system_gl.h)

<syntaxhighlight lang="c++">
#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>
</syntaxhighlight>

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 <code>std::unique_ptr</code> (or similar):

<syntaxhighlight lang="c++">
class CFileItem;

class Example
{
...
std::unique_ptr<CFileItem> m_fileItem;
}
</syntaxhighlight>

== Naming ==
=== Namespaces ===
Use upper case with underscores.
<syntaxhighlight lang="c++">
namespace KODI
{
[...]
}
</syntaxhighlight>

=== Constants ===
Use upper case with underscores.
<syntaxhighlight lang="c++">
constexpr int MY_CONSTANT = 1;
</syntaxhighlight>

=== Enums ===
Use PascalCase for the enum name and upper case with underscores for the values.
<syntaxhighlight lang="c++">
enum class Dummy
{
VALUE_X,
VALUE_Y
};
</syntaxhighlight>

=== Interfaces ===
Use PascalCase and prefix with an uppercase I. The filename has to match the interface name without the prefixed I, like this example from Logger.h:
<syntaxhighlight lang="c++">
class ILogger
{
public:
virtual void Log(...) = 0;
virtual ~ILogger() {}
}
</syntaxhighlight>

=== Classes ===
Use PascalCase and prefix with an uppercase C. Again, the filename has to match the class name without the prefixed C, as shown here from Logger.cpp:
<syntaxhighlight lang="c++">
class CLogger : public ILogger
{
public:
void Log(...) override;
}
</syntaxhighlight>

=== Methods ===
Use PascalCase always, uppercasing the first letter even if the methods are private or protected.
<syntaxhighlight lang="c++">
void MyDummyClass::DoSomething();
</syntaxhighlight>

=== Variables ===
Use CamelCase. Type prefixing (a/k/a Systems Hungarian notation) is discouraged.

==== Member variables ====
Prefix non-static member variables with <code>m_</code>. Prefix static member variables with <code>ms_</code>.
<syntaxhighlight lang="c++">
int m_variableA;
static int ms_variableB;
</syntaxhighlight>

==== Global variables ====
Prefix global variables with <code>g_</code>
<syntaxhighlight lang="c++">
int g_globalVariableA;
</syntaxhighlight>

{{Warning|Avoid use of globals as far as reasonably possible. We generally do not want to introduce new global variables.}}

== Comments ==
=== General ===
Use <code>// </code> for inline single-line and multi-line comments. Use <code>/* */</code> for the copyright comment at the beginning of the file. SPDX license headers are required for all code files (see example below).

✅ Good:
<syntaxhighlight lang="c++">
/*
* 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.
</syntaxhighlight>

❌ Bad:
<syntaxhighlight lang="c++">
/* A comment */
//another comment
</syntaxhighlight>

=== Doxygen ===
New 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 an exclamation point (<code>/*! */</code>) and backslash for doxygen commands (e.g. <code>\brief</code>).

✅ Good:
<syntaxhighlight lang="c++">
/*!
* \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);
</syntaxhighlight>

❌ Bad:
<syntaxhighlight lang="c++">
/**
* @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);
</syntaxhighlight>

== Logging ==
Use the provided logging function <code>CLog::Log</code>. Do not log to standard output or standard error using, for example, <code>printf</code> or <code>std::cout</code>.

The <code>Log</code> function uses the [https://fmtlib.net/ fmt library] for formatting log messages. Basically, you can use <code>{}</code> as a placeholder for anything and list the parameters to insert after the message, similar to <code>printf</code>. See [https://fmtlib.net/latest/syntax.html here] for the detailed syntax, and below for an example.

✅ Good:
<syntaxhighlight lang="c++">
CLog::Log(LOGDEBUG, "Window size: {}x{}", width, height);
</syntaxhighlight>

❌ Bad:
<syntaxhighlight lang="c++">
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
printf("Window size: %dx%d", width, height);
std::cout << "Window size: " << width << "x" << height << std::endl;
</syntaxhighlight>

The predefined logging levels are <code>DEBUG</code>, <code>INFO</code>, <code>NOTICE</code>, <code>WARNING</code>, <code>ERROR</code>, <code>SEVERE</code>, and <code>FATAL</code>. Use anything <code>INFO</code> 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. <code>DEBUG</code> messages are only written when debug logging is enabled.

== Classes ==
=== Member visibility ===
Make class data members <code>private</code>. Think twice before using <code>protected</code> for data members and functions, as its level of encapsulation is effectively equivalent to <code>public</code>.

=== Const correctness ===
Try to mark member functions of classes as <code>const</code> whenever and wherever reasonable.

=== Overriding virtual functions ===
When overriding virtual functions of a base class, add the <code>override</code> keyword. Do not add the <code>virtual</code> keyword.

✅ Good:
<syntaxhighlight lang="c++">
class CLogger : public ILogger
{
public:
void Log(...) override;
}
</syntaxhighlight>

❌ Bad:
<syntaxhighlight lang="c++">
class CLogger : public ILogger
{
public:
virtual void Log(...) override;
}
</syntaxhighlight>

=== Default member initialization ===
Use default member initialization instead of initializer lists or constructor assignments whenever it makes sense.
<syntaxhighlight lang="c++">
class Foo
{
bool bar{false};
};
</syntaxhighlight>

=== Destructors in interfaces ===
A class with any virtual functions should have a destructor that is either public and virtual, or else protected and non-virtual (cf. [https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-dtor-virtual ISO C++ guidelines]).

=== Constructor Initialization Lists ===
For lines up to [[#Line length]], everything stays on one line, excluding the braces, which must be on the adjacent lines in both directions.

<syntaxhighlight lang="c++">
MyClass::CMyClass(bool bBoolArg, int iIntegerArg) : m_bArg(bBoolArg), m_iArg(iIntegerArg)
{
}
</syntaxhighlight>

For longer lines, break before a colon and after a comma.
<syntaxhighlight lang="c++">
MyClass::CMyClass(bool bBoolArg,
int iIntegerArg,
const std::string& strSomeText,
const std::shared_ptr<CMyOtherClass>& myOtherClass)
: m_bBoolArg(bBoolArg),
m_iIntegerArg(iIntegerArg),
m_strSomeText(strSomeText),
m_myOtherClass(myOtherClass)
{
}
</syntaxhighlight>

== Other conventions ==
=== Output parameters ===
For functions that have multiple output values, prefer using a <code>struct</code> or <code>tuple</code> return value over output parameters that use pointers or references (cf. [https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-out-multi ISO C++ guidelines]). In general, try to avoid output parameters completely (cf. [https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-out ISO C++ guidelines]), [https://google.github.io/styleguide/cppguide.html#Output_Parameters 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, whereas return semantics make it clear what is happening.

=== Casts ===
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 <code>dynamic_cast</code> is used to cast to a pointer type, the result can be <code>nullptr</code> and needs to be checked accordingly.

=== NULL vs nullptr ===
Prefer the use of <code>nullptr</code> instead of <code>NULL</code>; <code>nullptr</code> is a type-safe version and as such, can't be implicitly converted to <code>int</code> or anything else.

=== Auto ===
Feel free to use <code>auto</code> wherever it improves readability, which is not as often the case as it may appear prima facie. Good places are iterators, or when dealing with containers, while bad places are in code that expects a certain type that is not immediately clear from the context.

✅ Good:
<syntaxhighlight lang="c++">
auto i = var.begin();

std::vector<CSizeInt> list;
for (const auto j : list)
{
[...]
}
</syntaxhighlight>

❌ Bad:
<syntaxhighlight lang="c++">
std::map<std::string, std::vector<int>>::iterator i = var.begin();
</syntaxhighlight>

=== For loops ===
Use range-based <code>for</code> loops wherever it makes sense. If iterators are used, see above about using [[#Auto]].
<syntaxhighlight lang="c++">
for (const auto& : var)
{
[...]
}
</syntaxhighlight>
Remove <code>const</code> if the value has to be modified. Do not use references to fundamental types that are not modified.

=== Include guards ===
Use <code>#pragma once</code>.

✅ Good:
<syntaxhighlight lang="c++">
#pragma once
</syntaxhighlight>

❌ Bad:
<syntaxhighlight lang="c++">
#ifndef SOME_FILE_H_INCLUDED
#define SOME_FILE_H_INCLUDED
[...]
#endif
</syntaxhighlight>

=== Type aliases ===
Use the C++ <code>using</code> syntax when aliasing types (highly encouraged when it improves readability).

✅ Good:
<syntaxhighlight lang="c++">
using SizeType = int;
</syntaxhighlight>

❌ Bad:
<syntaxhighlight lang="c++">
typedef int SizeType;
</syntaxhighlight>

=== Goto ===
The usage of <code>goto</code> is discouraged.

=== Macros ===
Try to avoid using C macros; in many cases, they can be easily substituted with other non-macro constructs.

=== Constexpr ===
Prefer <code>constexpr</code> over <code>const</code> for constants when possible. Try to mark functions <code>constexpr</code> when reasonable.

[[Category:Development]]

Contributing code - Revision history

RogueScholar: Add toclimit-2 class to TOC div tag

RogueScholar: Add current coding conventions to eponymous article on wiki