Skip to content

Cpp Coding Conventions

Jump to bottom
Alexandr Akulich edited this page Nov 11, 2015 · 5 revisions

Table of Contents

Common things

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

Naming conventions:

  • Do choose easily readable identifier names. For example, a variable named horizontalAlignment is more readable in English than alignmentHorizontal.
  • Do not use Hungarian notation except in two cases:
    • UI-related code, i.e. btnOpen, lblMessage, chkRecursive, cbGameType
    • Interface classes: IClient, IGameLevel, IGamePersistent, etc

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
  • Functions: PascalCase
  • Public and protected fields: PascalCase
    • Math primitives can use lowercase: vector2f.x, angle3f.yaw, matrix44f.m03
  • 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(&params, 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;

Header guards

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

End of file

Files should terminate with an empty line:

BAD

int main()
{
    return 0;
}<EOF>

GOOD

int main()
{
    return 0;
}
<EOF>

Conditional compilation and preprocessor directives

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");

File naming and mandatory content

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>

Comments of XXX should be : . 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

Home
English - Русский - Українська - Беларуская - Polski - Český

Home

Clone this wiki locally