Skip to content

Cpp Coding Conventions

Jump to bottom
Pavel Kovalenko 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 <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

Home
English - Русский - Українська - Polski - Český - Français - Deutsch

Home

Clone this wiki locally