Cpp Coding Conventions - OpenXRay/xray-16 GitHub Wiki
- Common things
- Naming conventions
- Header guards
- End of file
- Conditional compilation and preprocessor directives
- File naming and mandatory content
Shift width (indentation) is 4 spaces.
Line size is 120 columns.
Indentation is always one shift-width (4):
BAD
GatherInfo(expression, description, arg0, arg1, file, line,
function, assertionInfo);
GOOD
GatherInfo(expression, description, arg0, arg1, file, line,
function, assertionInfo);
Open and close brackets of a section should be on the same level:
BAD
if (lastErr==ERROR_SUCCESS) {
*buffer = 0;
return;
}
GOOD
if (lastErr==ERROR_SUCCESS)
{
*buffer = 0;
return;
}
Access modifiers are indented like goto labels - at the class level indentation:
BAD
class ObjectComparer
{
public:
ObjectComparer();
int Compare(const Object &a, const Object &b);
private: int InternalCompare(const Object &a, const Object &b);
};
GOOD
class ObjectComparer
{
public:
ObjectComparer();
int Compare(const Object &a, const Object &b);
private:
int InternalCompare(const Object &a, const Object &b);
};
Namespace content should not be indented:
BAD:
namespace XRay
{
namespace Math
{
#define PI 3.14159265359
}
}
GOOD
namespace XRay
{
namespace Math
{
#define PI 3.14159265359
}
}
When breaking up a long line, it should continue with one shift-width for indentation:
BAD
if (lineLength>1 && (screenSize.Vertical<bufferSize.Vertical
|| explicitLines))
{
printf("this is a test section that will show how to handle "
"long lines, such as %s which is %d lines long",
"this", 2);
SetWindowText( hWnd,
"Button Name" );
}
GOOD
if (lineLength>1 && (screenSize.Vertical<bufferSize.Vertical
|| explicitLines))
{
printf("this is a test section that will show how to handle "
"long lines, such as %s which is %d lines long", "this", 2);
SetWindowText(hWnd, "Button Name");
}
if/for/while statement that takes more than one line should always have brackets same thing goes when the then/loop part is more than one line:
BAD
if (!ErrorAfterDialog)
MessageBox(NULL, "Fatal error occured\n\n"
"Press OK to abort program execution", "Fatal error",
MB_OK|MB_ICONERROR|MB_SYSTEMMODAL);
GOOD
if (!ErrorAfterDialog)
{
MessageBox(NULL, "Fatal error occured\n\n"
"Press OK to abort program execution", "Fatal error",
MB_OK|MB_ICONERROR|MB_SYSTEMMODAL);
}
Function prototypes should be spaced like function calls:
BAD
void Debug::Backend(const char *reason,
const char *expression,
const char *arg0,
const char *arg1,
const char *file,
int line,
const char *function,
bool &ignoreAlways);
{
// function code
}
BAD
void Debug::Backend(const char *reason, const char *expression, const
char *arg0, const char *arg1, const char *file, int line, const char *
function, bool &ignoreAlways);
GOOD
// declaration
static void Backend(const char *reason, const char *expression,
const char *arg0, const char *arg1, const char *file, int line,
const char *function, bool &ignoreAlways);
// implementation
void Debug::Backend(const char *reason, const char *expression,
const char *arg0, const char *arg1, const char *file, int line,
const char *function, bool &ignoreAlways)
{
// function code
}
// if you need to comment parameters
void Debug::Backend(
const char *reason, // error reason
const char *expression, // failed expression
const char *arg0, // first argument
const char *arg1, // second argument
const char *file,
int line,
const char *function,
bool &ignoreAlways) // ignore errors of this type
{
// function code
}
switch statements should have the case on the same level, no space before ':'.
if the case is one line, then one space after ':':
BAD
switch (vendor)
{
case Vendor::Intel :
...
case Vendor::AMD :
...
}
GOOD
switch (vendor)
{
case Vendor::Intel:
...
case Vendor::AMD:
...
}
GOOD
switch (controlId)
{
case IDC_TOGGLESTATS:
app->drawStats = !app->drawStats;
break;
case IDC_RESETCAMERA:
app->ResetCamera();
break;
case IDC_LIGHT_SIZE:
{
auto r = hud->GetSlider(IDC_LIGHT_SIZE)->GetValue()*0.01f;
app->spotLightObj->SetLightRadius(r);
break;
}
}
GOOD
switch (vendorId)
{
case 0x756e6547: vendor = Vendor::Intel; break;
case 0x68747541: vendor = Vendor::AMD; break;
default: vendor = Vendor::Unknown; break;
}
No space between function name and opening brackets, no space between opening brackets and first parameter, one space after comma:
BAD
Msg ("hello %s\n", "world");
Msg( "hello world\n" );
Msg("hello world\n","world");
GOOD
Msg("hello world\n", "world");
One space after reserved words before the opening parenthesis:
BAD
if(OnDialog)
GOOD
if (OnDialog)
'then' part of if statement should be in a separate line. Reason: allows to debug the if and see when it is run.
BAD
if (OnDialog) OnDialog(true);
GOOD
if (OnDialog)
OnDialog(true);
else if statements should be on the same level at the starting if
Reason: this is similar to switch statement.
BAD
if (!strcmp(argv[1],"--help")) PrintUsage();
else if (!strcmp(argv[1], "--run")) {
RunApplication();
PrintResults();
} else PrintError();
GOOD:
if (!strcmp(argv[1], "--help"))
PrintUsage();
else if (!strcmp(argv[1], "--run"))
{
RunApplication();
PrintResults();
}
else
PrintError();
return statements should not have parentheses and should not have a space after them:
BAD
return (0);
GOOD
return 0;
BAD
return ;
GOOD
return;
Don't use unneeded parentheses in expressions:
BAD
if ((a!=b) || (c!=d))
...
GOOD
if (a!=b || c!=d)
...
Don't call return at the end of a function returning void:
BAD
void foo()
{
bar();
return;
}
GOOD
void foo()
{
bar();
}
Don't separate code sections with more than one blank line.
class/struct/enum definitions, initializations should open { at the next line:
BAD
struct Size {
int Width;
int Height;
};
GOOD
struct Size
{
int Width;
int Height;
};
GOOD (no members)
struct Dummy {};
BAD
Size size = {
LayoutHelper::AdjustLength(Clamp(length, 0, 1024)),
LayoutHelper::CalculatePreferredHeight(length, scaleFactor.y)
};
GOOD
Size size =
{
LayoutHelper::AdjustLength(Clamp(length, 0, 1024)),
LayoutHelper::CalculatePreferredHeight(length, scaleFactor.y)
};
GOOD (initialization fits in a single line)
Size size = {1024, 16};
Prefer C++ comments to C comments:
GOOD
/* draw overlay statistics */
DrawStats();
GOOD (better)
// draw overlay statistics
DrawStats();
Comments should be aligned as the code they comment, or one space after the end of line:
BAD
/*
* draw overlay statistics
*/
DrawStats();
BAD
//
// draw overlay statistics
//
DrawStats();
GOOD
/* draw overlay statistics */
DrawStats();
GOOD (better)
// draw overlay statistics
DrawStats();
GOOD
DrawStats(); // draw overlay statistics
Comments which occupy more than one line should adhere to the following guidline:
BAD
//
// last render stage: draw overlay engine performance statistics
// (input, render, sound, ai, physics)
//
DrawStats();
GOOD
// last render stage: draw overlay engine performance statistics
// (input, render, sound, ai, physics)
DrawStats();
Common sense should come into play when deciding whether to use for or while loops:
BAD
int i = 0;
while (i<10)
{
i++;
...
}
GOOD
for (int i = 0; i<10; i++)
...
BAD
for (ptr = ptr->next; ptr; ptr = ptr->next)
...
GOOD
while (ptr = ptr->next, ptr)
...
In for and while loops without a statement, statement separator should be in the same line.
BAD
for(i = 0; Test(i); i++)
;
GOOD
for (i = 0; Test(i); i++);
Put one space before and after an assignment operator:
BAD
int a=0;
x= x+1;
capacity*=2;
GOOD
int a = 0;
x = x+1;
capacity *= 2;
Don't put a space before a statement separator, put one after it:
BAD
for (i = 0 ;i<10 ;i--, j *= 2) ;
GOOD
for (i = 0; i<10; i--, j *= 2);
Don't put a space after increment and decrement. Increment after the value, not before.
BAD
i --;
++j;
GOOD
i--;
j++;
Trivial > >= < <= == != expressions should not have spaces around them.
Examples: a, array[i].var[j], sizeof(Foo), x+1
Pointers (->
) are not counted as trivial expressions for > >= < <=
, since it's
hard to read x->y>z
, while its easy to read x->y==z
.
BAD
if (x > 5)
GOOD
if (x>5)
BAD
if (x+1>5)
GOOD
if (x+1 > 5)
BAD
if (f(x, y) > g(y, z))
GOOD
if (f(x, y)>g(y, z))
BAD
if (x->y == 5)
GOOD
if (x->y==5)
BAD
if (x->y>=5)
GOOD
if (x->y >= 5)
Don't increment, set or change value of variable inside a function call:
BAD
CalcXY(i++, 2);
BAD
SetX(i = 3);
GOOD (if i is not a global)
CalcXY(i, 2);
i++;
GOOD (if i is a global that CalcXY() uses)
i++;
CalcXY(i-1, 2);
GOOD
if (i++)
a[i++] = 4;
GOOD (if i is not a global)
i = 3;
SetX(i);
In for loops, when there is a function that gets the next element, it should be done once (inside the step condition):
BAD:
for (int i = 0, ch = GetChar(); ch=='\r'; ch = GetChar(), i++)
HandleResult(ch);
GOOD:
for (int i = 0; (ch = GetChar())=='\r'; i++)
HandleResult(ch);
When assigning in a truth value (if/for/while), use the following pattern in order to eliminate compiler warnings:
BAD:
for (int i = 0; ch = GetChar(); i++)
HandleResult(ch);
if (x = ComputeNum())
return x;
GOOD:
for (int i = 0; ch = GetChar(), ch; i++)
HandleResult(ch);
if (x = ComputeNum(), x)
return x;
In string initialization and null termination, you must use 0:
BAD:
string[0] = '\0';
if (string[3]=='\0')
{
}
GOOD:
*string = 0;
if (!string[3])
{
}
Naming conventions:
- Do choose easily readable identifier names. For example, a variable named
horizontalAlignment
is more readable in English thanalignmentHorizontal
. - Do not use Hungarian notation except in the following cases:
- UI-related code, i.e.
btnOpen
,lblMessage
,chkRecursive
,cbGameType
- Interface classes:
IClient
,IGameLevel
,IGamePersistent
, etc - Template type arguments:
TBase
,TResult
- UI-related code, i.e.
Casing styles:
- class/struct/enum:
PascalCase
- Math primitives can be in lowercase:
vector2f
,angle3f
,matrix44f
- Using declarations of primitive types can be in
lowercase
:uint
,int16
,uint64
- Math primitives can be in lowercase:
- Functions:
PascalCase
- Public and protected fields:
PascalCase
- Math primitives can use
lowercase
:vector2f.x
,angle3f.yaw
,matrix44f.m03
- Math primitives can use
- Private fields:
camelCase
- Local variables:
camelCase
- Global variables:
PascalCase
- Namespaces:
PascalCase
Don't put inline qualifier on a function defined within a class definition - such function is inline according to the standard.
When defining an interface class, declare destructor as pure virtual and add its empty inline implementation after the class definition:
GOOD
class IServer
{
public:
virtual ~IServer() = 0;
};
inline IServer::~IServer() {}
Do not mark interface classes with linkage attributes.
When overriding virtual function, use override specifier on function declaration.
BAD
class Server : public IServer
{
...
virtual void OnClientConnected(IClient *client);
...
};
GOOD
class Server : public IServer
{
...
virtual void OnClientConnected(IClient *client) override;
...
};
Checking function return values: Functions that return bool or a pointer will be checked without comparing, if they return true or false.
BAD
if (strstr(args, " -h")!=nullptr)
PrintHelp();
GOOD
if (strstr(args, " -h"))
PrintHelp();
BAD
if (IsUpdateRequired()==true)
Update();
GOOD
if (IsUpdateRequired())
Update();
BAD
cl = GetClient(id);
if (cl==nullptr)
return nullptr;
GOOD
cl = GetClient(id);
if (!cl)
return nullptr;
BAD
cl = GetClient(id);
if (cl!=nullptr)
Disconnect(cl);
GOOD
cl = GetClient(id);
if (cl)
Disconnect(cl);
When allocating a structure on stack, use zero initializer unstead of memset:
BAD
ShaderParams params;
memset(¶ms, sizeof(params), 0);
GOOD
ShaderParams params = {};
When possible, prefer inline functions to macros:
BAD
#define RAD2DEG(angle) ((angle)*180/PI)
GOOD
template <typename T>
T RadiansToDegrees(T angle) { return angle*180/PI; }
Macro names should normally be all upper case, with normal spacing as rest of code:
BAD
#define RAD2DEG( angle ) ((angle)*180/PI)
#define RAD2DEG(angle) \
((angle) * 180 / PI)
GOOD
#define RAD2DEG(angle) ((angle)*180/PI)
GOOD
#define RAD2DEG(angle) \
((angle)*180/PI)
Common use macros that are more than one statement long should be encapsulated by do...while(false). This enables using them inside if...else statements:
Example usage:
if (condition)
DO_A_AND_B;
else
DoSomethingElse();
BAD
#define DO_THIS_AND_THAT() \
{ \
DoThis(); \
DoThat(); \
}
GOOD
#define DO_THIS_AND_THAT() \
do { \
DoThis(); \
DoThat(); \
} while (false)
Don't use forward declarations unless they are necessary. Instead, change the order so the caller is below the callee.
BAD
void Callee();
void Caller()
{
Callee();
}
void Callee()
{
}
GOOD
void Callee()
{
}
void Caller()
{
Callee();
}
Prefer portable data types from Common.hpp over OS specific data types:
BAD
BYTE b;
DWORD w;
GOOD
byte b;
uint32 w;
At the beginning of a file add inclusion protection. Don't use include guards. Instead, use #pragma once - it's less prone to making mistakes and it is less code to type. Though it's not a part of the standard, it's well supported across compilers.
BAD
#ifndef _XRAY_HPP_
#define _XRAY_HPP_
// The XRay.hpp content comes here
#endif // _XRAY_HPP_
GOOD
#pragma once
// The XRay.hpp content comes here
Files should terminate with an empty line:
BAD
int main()
{
return 0;
}<EOF>
GOOD
int main()
{
return 0;
}
<EOF>
COMPLEX #if sections should adhere to the following guideline:
BAD
#ifndef NO_SINGLE
Msg("* Found new patch: %s", versionName);
#ifdef DOWNLOAD_UPDATES
#ifdef DOWNLOAD_UPDATES_GATHER_STATS
DownloadStats stats;
DownloadUpdate(downloadUrl, stats);
stats.Dump();
#else
DownloadUpdate(downloadUrl);
#endif
#endif
#endif
BAD
#ifndef NO_SINGLE
Msg("* Found new patch: %s", versionName);
# ifdef DOWNLOAD_UPDATES
# ifdef DOWNLOAD_UPDATES_GATHER_STATS
DownloadStats stats;
DownloadUpdate(downloadUrl, stats);
stats.Dump();
# else
DownloadUpdate(downloadUrl);
# endif
# endif
#endif
GOOD
#ifndef NO_SINGLE
Msg("* Found new patch: %s", versionName);
#ifdef DOWNLOAD_UPDATES
#ifdef DOWNLOAD_UPDATES_GATHER_STATS
DownloadStats stats;
DownloadUpdate(downloadUrl, stats);
stats.Dump();
#else
DownloadUpdate(downloadUrl);
#endif
#endif
#endif
SIMPLE #if sections should not be indented:
GOOD
#ifndef DEBUG
#ifdef _DEBUG
#define DEBUG
#endif
#ifdef MIXED
#define DEBUG
#endif
#endif
BAD
#ifndef DEBUG
#ifdef _DEBUG
#define DEBUG
#endif
#ifdef MIXED
#define DEBUG
#endif
#endif
GOOD
void Sleep(int milliseconds)
{
#ifdef WINDOWS
::Sleep(milliseconds);
#else
::sleep(milliseconds);
#endif
}
BAD
void Sleep(int milliseconds)
{
#ifdef WINDOWS
::Sleep(milliseconds);
#else
::sleep(milliseconds);
#endif
}
After #endif you can put a comment telling to what if it belongs if there is a large gap between the #if and its corresponding #endif.
BAD
#ifdef _EDITOR
VerifyPath(path);
#endif // _EDITOR
GOOD
#ifdef _EDITOR
VerifyPath(path);
#endif
GOOD
#ifdef _EDITOR
// Lots of editor related code, that you really have to scroll down
// to see all of it.
#endif // _EDITOR
Only use the defined() macro when you have a complex expression. If you find that you do need to use the defined() macro for more than one flag, see if that flags can be grouped under another one new flag:
BAD
#ifndef DEDICATED_SERVER
#ifdef CONFIG_SHOW_LOGO_WINDOW
DestroyWindow(logoWindow);
#endif
#endif
BAD
#if defined(CONFIG_SHOW_LOGO_WINDOW)
DestroyWindow(logoWindow);
#endif
GOOD
#ifdef CONFIG_SHOW_LOGO_WINDOW
DestroyWindow(logoWindow);
#endif
GOOD (since you really need both flags)
#if !defined(DEDICATED_SERVER) && defined(CONFIG_SHOW_LOGO_WINDOW)
DestroyWindow(logoWindow);
#endif
Use using
instead of typedef
:
BAD
typedef void (*FunctionType)(double);
typedef Vector3<float> vector3f;
GOOD
using FunctionType = void (*)(double);
using vector3f = Vector3<float>;
Use strongly typed enums instead of plain C enums:
BAD
enum CmdStatus
{
CmdStatusOk,
CmdStatusInProgress,
CmdStatusFailed,
};
GOOD
enum class CmdStatus
{
Ok,
InProgress,
Failed,
};
Enums that can be serialized should have values assigned:
BAD
enum class CmdStatus
{
Ok = 1,
InProgress,
Failed,
};
GOOD
enum class CmdStatus
{
Ok = 1,
InProgress = 2,
Failed = 3,
};
Every enum line have to be comma terminated - this makes the enum extensible. Only separate the last line, if it is meant to never be surpassed.
BAD
enum class CmdType
{
Reset = 1,
Load = 2,
Save = 3
};
GOOD
enum class CmdType
{
Reset = 1,
Load = 2,
Save = 3,
};
GOOD
enum class CmdType
{
Reset = 1,
Load = 2,
Save = 3,
Invalid = 0xFF
};
Put the pointer and reference next to the identifier:
BAD
IClient* GetServerClient();
void BanAddress(const IPAddress& ip);
GOOD
IClient *GetServerClient();
void BanAddress(const IPAddress &ip);
Put static/const/volatile/mutable before the type:
BAD
int const static z;
bool volatile exit;
int static Log(char const *s);
bool mutable lastState;
char const* const* name;
GOOD
static const int z;
volatile bool exit;
static int Log(char const *s);
mutable bool lastState;
const char *const *name;
Use stack allocation for local variables of constant size. Use dynamic allocation only when needed.
BAD
ConnectionParams *cp = new ConnectionParams();
...
delete cp;
GOOD
ConnectionParams cp;
Operator delete also handles nullptr, so no need for 'if ()' before it:
BAD (if str can be null):
if (str)
delete str;
GOOD:
delete str;
Constant strings longer than one line should be closed on each line by a quote and opened again on the next line. Words should have the space after them on the same line.
BAD
Log("Hello world, this is a long string that we want to print and
is more than 120 chars long so we need to split it");
BAD
Log("Hello world, this is a long string that we want to print"
" and is more than 120 chars long so we need to split it");
GOOD
Log("Hello world, this is a long string that we want to print "
"and is more than 120 chars long so we need to split it");
Functions that accept zero parameters should be defined without void:
BAD
IClient *GetServerClient(void);
GOOD
IClient *GetServerClient();
Trivial + - * / expressions should not have spaces around them:
BAD
Foo(a + b);
GOOD
Foo(a+b);
BAD
Foo(2 * (a + b));
GOOD
Foo(2*(a+b));
BAD
Foo((a + b) / (1000 * 1000));
GOOD
Foo((a+b)/(1000*1000));
BAD
Foo((Min(aMin, bMin)+Max(aMax, bMax))*(width+Pow(a, p))/(1000*1000));
GOOD
Foo((Min(aMin, bMin)+Max(aMax, bMax)) * (width+Pow(a, p)) / (1000*1000));
Don't put empty lines between code inside functions. If you want to separate code fragments, put a comment line.
BAD
for (auto &item : items)
storage.push_back(item);
if (!storage.size())
Log("! ERROR: No items found");
GOOD
for (auto &item : items)
storage.push_back(item);
// check if there's no items
if (!storage.size())
Log("! ERROR: No items found");
Class Foo
in src/Bar/Foo.cpp
& Foo.hpp
.
All files should follow this exact example.
Unless using precompiled headers, #include "Config.hpp"
must be first in
the *.cpp
file to make sure the features configuration affect all the rest of
the files. If using precompiled headers, #include "Config.hpp"
must appear
immediately after the precompiled header file (e.g. stdafx.hpp
).
Header files must be self contained, i.e. they must be able to compile without relying on another include line to come before them.
Therefore #include "Foo.hpp"
must be immediately after to make sure Foo.hpp
compiles standalone without any dependency on includes before it.
GOOD
Foo.hpp template
#pragma once
#include "Config.hpp"
// ... Put here minimal includes required by Foo.hpp ...
// ... Put here your declarations ...
Foo.cpp template
#include "Config.hpp"
#include "Foo.hpp"
// ... Put here minimal includes required by Foo.cpp ...
// ... Put here your code ...
GOOD (using precompiled headers)
stdafx.hpp template
#pragma once
#include "Config.hpp"
// ... Put here additional includes ...
Foo.hpp template
#pragma once
#include "Config.hpp"
// ... Put here minimal includes required by Foo.hpp ...
// ... Put here your declarations ...
Foo.cpp template
#include "stdafx.hpp"
#include "Config.hpp"
#include "Foo.hpp"
// ... Put here minimal includes required by Foo.cpp ...
// ... Put here your code ...
Use #include "Foo.hpp"
for headers in the same directory as the source
including them.
Use #include "PathToFoo/Foo.hpp"
for headers in other directories, where
PathToFoo is relative to root engine directory.
Use #include <Foo.hpp>
for external system files.
BAD
#include <Config.hpp>
#include <xrCore/xrCore.hpp>
#include "IReader.hpp"
#include "algorithm"
GOOD
#include "Config.hpp"
#include "xrCore.hpp" // we are in src/xrCore directory
#include "IO/IReader.hpp" // from src/xrCore/IO/IReader.hpp
#include <algorithm>
Free function used in one CPP file: should be implemented static, and a declaration in the CPP file should only be added if used before implementation.
Free function used out of the CPP file (global): should be declared in the HPP file. class/struct used in one CPP file: should be declared inside the CPP file.
A pointer is needed out of the CPP file: a forward declaration should be in the HPP file, and the definition should be in the C file.
class/struct members are needed out of the CPP file: declare it in the HPP file.
Comments of XXX
should be <keywords>: <comment>
. Multiple names should be
separated with a '/':
BAD
XXX: dima: optimize this case
XXX alexmx: RENDER check if already loaded
XXX alexmx: oles: move to common
GOOD
XXX: optimize this case
XXX dima: optimize this case
XXX alexmx RENDER: check if already loaded
XXX alexmx/oles: move to common