-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Coding Conventions
Everybody has their own preferred style for writing code, however, in the interest of making the codebase easier to maintain, we request that the following conventions be observed. Code that is lifted from other projects does not need to be modified to match these rules – no need to fix what isn't broken.
This convention page follows the requirements keywords of RFC 2119. Requirements are seen as CAPITAL letter words.
The majority of LMMS conventions follows the Qt Coding Style. Some conventions are repeated here, and if not noted here MAY fall under the Qt style. The C++ Core Guidelines also contain good advice, and MAY be referenced in situations not covered by these conventions.
Header guards MUST NOT begin with an underscore _
. Identifiers that begin with an underscore followed by a CAPITAL letter are reserved identifiers in C++. If you edit an older file which contains an improper header guard, please fix it to comply with guidelines.
Header guards should also prepend the namespace of the class the file declares. For example:
// SongEditor.h
#ifndef LMMS_GUI_SONG_EDITOR_H
#define LMMS_GUI_SONG_EDITOR_H
namespace lmms::gui
{
class SongEditor;
} // namespace lmms::gui
#endif // LMMS_GUI_SONG_EDITOR_H
The first #include
in a cpp
file MUST be its own related header file. Afterwards group #include
s by type, where groups are separated by a newline, and ordered by name.
// MySourceFile.cpp
#include "MySourceFile.h"
#include <algorithm>
#include <vector>
#include <QMap>
#include <QString>
#include "DataFile.h"
#include "Engine.h"
#include "GuiApplication.h"
Types MUST begin with an uppercase letter, unless a lowercase letter is required for the code to function.
class ResourcesDB;
enum class MyEnum
{
// ...
};
using AutoModelList = std::vector<AutomatableModel*>;
struct MyCustomDeleter
{
using pointer = int; // Okay, since unique_ptr requires this specific type name
void operator()(int);
};
using MyCustomPointer = std::unique_ptr<int, MyCustomDeleter>;
Variables MUST use camel case format, for example: nextValue
.
- Non-static member variables MUST be prefixed with
m_
- Static member variables MUST be prefixed with
s_
The remaining variable name MUST follow the Variable Names rule.
float m_currentValue;
static int s_quantization;
The exposure of data members MUST be minimized by choosing more restrictive access specifiers and providing getters/setters if needed.
Exceptions:
- Public non-static member variables of POD structs with no member functions (except perhaps constructors) which are only ever accessed in a way which makes their member status obvious. These variables SHALL use camel case format in order to limit visual clutter, for example:
sampleRate
. - Public static constants. Public static constants SHALL use
UpperCamelCase
.
struct MyPair
{
static constexpr int MyPublicConstant = 123; // GOOD
static constexpr int s_myPublicConstant = 123; // BAD
int firstMember; // GOOD
private:
int secondMember; // BAD
};
Global scope constants SHALL use UpperCamelCase
.
Some older code prefixed function parameters with an underscore _
. Function parameters SHOULD NOT be prefixed with an underscore.
Namespaces MUST use lower_snake_case
.
Every line of text SHOULD be at most 120 characters long.
Tabs MUST be used for indentation. Instructions for configuring QtCreator can be found here.
Flow control statements (if
, else if
, else
, for
, do
, while
, and switch
) MUST have a space between the keyword and the opening parenthesis.
You MUST use the true
/false
keywords instead of non-standard macros or integers.
b = TRUE; // BAD
b = true; // GOOD
b = 1; // BAD
You MUST use the nullptr
keyword instead of macros or integers.
p = NULL; // BAD
p = nullptr; // GOOD
p = 0; // BAD
The ternary operator ? :
SHOULD be used only when conditionally selecting one of two values. It SHOULD NOT be used when the value of the expression is not used.
// BAD - result begins uninitialised and cannot be made const. Ample opportunity for bugs.
int value;
if (useA()) { value = getA(); }
else { value = getB(); }
// GOOD - can initialize the result variable at declaration and make it const.
const auto value = useA() ? getA() : getB();
// BAD - result is unused, code is unclear
hasError() ? throw error{"oh no"} : doSomethingElse();
// GOOD - clear
if (hasError()) { throw error{"oh no"}; }
doSomethingElse();
- Spaces MUST NOT be added after an opening parenthesis or bracket
- Spaces MUST NOT be added before a closing parenthesis or bracket
- Flow control statements (
if
,else if
,else
,for
,do
,while
, andswitch
) MUST use explicit blocking - Block braces SHOULD be on their own line
// Spaces before/after parentheses and brackets
void doThis( int a ); // BAD
void doThis(int a); // GOOD
if ( m_sample > 0 ) // BAD
if (m_sample > 0) // GOOD
array[ offset ] // BAD
array[offset] // GOOD
// BAD - Example of not explicit blocking
if (m_sample > 0)
--m_sample;
// GOOD - Example of explicit blocking
if (m_sample > 0)
{
--m_sample;
}
// If the block can fit on one line, it is acceptable to format it as shown below.
// (Note that rules 1 and 2 do not apply, the contents are separated by a space on either side.)
if (m_sample > 0) { --m_sample; }
Return statements SHOULD NOT have parentheses around the returned value.
return (bar); // BAD
return bar; // GOOD
Spaces MUST NOT be added before semicolons.
return this ; // BAD
return this; // GOOD
Colons MUST be surrounded by spaces in range-based for loops and in base class specifications. Colons MUST NOT be preceeded by space when following a label or access specifier.
class Foo: public Bar // BAD
class Foo : public Bar // GOOD
{
public : // BAD
public: // GOOD
void foo()
{
for (const auto bar: m_baz) // BAD
for (const auto bar : m_baz) // GOOD
{
switch (bar)
{
default : // BAD
default: // GOOD
break;
}
}
}
};
Spaces MUST be used before and after infix operators (=
, +
, -
, *
, /
, etc.).
sub_note_key_base=base_note_key+octave_cnt*NOTES_PER_OCTAVE; // BAD
sub_note_key_base = base_note_key + octave_cnt * NOTES_PER_OCTAVE; // GOOD
When wrapping long expressions, infix operators SHOULD be placed at the beginning of the line, not the end.
someVariable = someLongNameThatMeansTheLineNeedsWrapping -
someOtherRatherLongName +
iCantThinkOfWhatToCallThisOne; // BAD
someVariable = someLongNameThatMeansTheLineNeedsWrapping
- someOtherRatherLongName
+ iCantThinkOfWhatToCallThisOne; // GOOD
If long ternary expressions don't fit on one line, they MUST be formatted as one of the below, depending on the readability of the surrounding code. If the expressions are very long or convoluted, you SHOULD use if/else blocks instead.
a == condition
? value
: otherValue;
// OR, depending on surrounding code readability
a == condition
? value
: otherValue;
Single-line doxygen comments SHOULD use //! ...
for comments about the following line, and //!< ...
for comments about the current line. Multi-line doxygen comments SHOULD use /** ... */
.
/**
this
is a
larger
comment
about `class S`
*/
class S
{
//! this comment is above `f` and explains what `f` does
void f();
int m_i; //!< this comment comes after `m_i` and explains what `m_i` does
};
For referencing code, you can use backticks, like above, or e.g. @p
to reference a parameter name.
Doxygen comments SHOULD be used when writing documentation that is relevant when using the documented entity elsewhere in the codebase. Examples include: the purpose of a function and what its parameters mean, what the data in a member variable means, what a particular class represents, or what sort of functions are contained in a namespace.
Doxygen comments SHOULD NOT be used for comments that are only relevant in the context of the code that contains them. Examples include: what the purpose of a local variable in a function is, details of how a particular algorithm works, or a note indicating that a piece of code needs refactoring.
Most comments in headers will be candidates for doxygen-style comments.
When submitting a PR, normal comments relevant to the modified code should be turned into doxygen comments if appropriate. For example, if you see code like
// This function does ...
void function();
then change it to
//! This function does ...
void function();
However, changes must be done with care: in the following examples, blindly replacing //
by //!
would lead to bad doxygen comments:
// the next two functions do ...
void f();
void g();
// This class could need some cleanup
class c { /* ... */ };
Sometimes it's worth merging code even if there are improvements to be made. In these cases its helpful to leave comments highlighting what improvements should be made and where. For the sake of discoverability, please use a searchable keyword in these comments. Some keywords currently used in the codebase are TODO
, HACK
, Workaround
, and FIXME
. An example from PR #5427: // TODO: We should do this work outside the event thread
.
If a piece of code is only relevant for some versions of Qt, QT_VERSION
and QT_VERSION_CHECK(MAJ, MIN, PAT)
can be used to exclude it in other versions. This has the added benefit that when we update our minimum Qt version, a simple search for QT_VERSION
can be used to remove outdated code. A simplified example based on PR #5777:
#if QT_VERSION < QT_VERSION_CHECK(5, 12, 2)
// Your code here
#endif
Changes to the C++ standard can simplify or otherwise improve existing code. Since we lag behind the standards, sometimes we can know ahead of time that a particular section of code could be better written if a newer standard were used. In this case, a comment along the lines of // TODO C++##:
should be added. An example from PR #5589: // TODO C++17 and above: use std::optional instead