User Tools

Site Tools


best_practices

Best Practices

General

Tips & tricks

Always initialize primitive variables (other types do not require this)

// Bad
int myVariable;
std::string myString;
 
// Good
int myVariable{};
std::string myString; // std::string is not a primitive variable, so initialization is optional

Always set parameters as const reference if they are non-primitives

// Bad
void myFunction(int firstParameter, std::string secondParameter)
{
    //...
}
 
// Good
void myFunction(int firstParameter, const std::string &secondParameter)
{
    //...
}

Do not use "using namespace" outside of a function => improves readability (and only use it when needed)

// Bad
using namespace std;
 
void myFunction()
{
    string myString;
    //...
}
 
// Good
void myFunction()
{
    using namespace std;
 
    string myString;
    //...
}
 
// Better
void myFunction()
{
    std::string myString;
    //...
}

Note that you can use namespace aliases to simplify a complex namespace hierarchy:

namespace myNamespace = some::complex::namespace::hierarchy;

Use references instead of pointers if possible => easier to use

(but note that Qt uses a lot of pointers for historical reasons)

// Bad
void myFunction(MyObject *object)
{
    object->function();
    //...
}
 
// Good
void myFunction(MyObject &object)
{
    object.function();
    //...
}

Use smart pointers instead of raw/naked/dumb pointers => easier to use

(but note that Qt uses its own memory management system, so if using Qt classes or Qt-based classes you will have to use new, see this section)

// Bad
MyObject *object = new MyObject;
 
delete object;
 
// Good
#include <memory>
 
std::unique_ptr<MyObject> object{std::make_unique<MyObject>()};
 
// Good
#include <QObject>
 
// parentObject is a pointer to a QObject
 
MyObject *object = new MyObject(parentObject);

Note the use of std::make_unique here; it should be used instead of new when using std::unique_ptr.

Never mix C and C++ code

C and C++ are very different languages. The fact that most of the C language can be compiled using a C++ compiler does not mean that you should mix both. For example, C-arrays are difficult to use correctly and should be avoided. Prefer using QList when writing Qt-based code and std::vector otherwise.

// Bad
int *myArray = new int[42];
 
delete[] myArray ;
 
// Good
#include <QList>
 
QList<int> myArray; // QList has no resize() function, so reserve() could be used to reserve memory and append() or push_back() could be used to add elements
 
// Good
#include <vector>
 
std::vector<int> myArray(42);

Static arrays should use std::array. They offer the same performance while adding convenience functions like size().

// Bad
int myArray[42];
 
// Good
#include <array>
 
std::array<int, 42> myArray;

In addition, note that C code relying on functions to free memory like free(), are not exception safe. This means that mixing exception-throwing code with these functions would trigger memory leaks in case an exception is thrown.

// Bad
int* stuff = (int*)malloc(sizeof(int) * 42);
 
QPushButton *button = new QPushButton(parent); // What happens if new triggers an exception? "stuff" is never freed and a memory leak occurs
 
free(stuff);
 
// Good
#include <array>
 
std::array<int, 42> stuff;
 
QPushButton *button = new QPushButton(parent); // "stuff" never leaks memory

If you have to write a function that returns multiple values, prefer returning a std::tuple instead of using reference parameters (if possible)

// Bad
void myFunction(int &outFirstVariable, std::string &outSecondVariable)
{
    outFirstVariable = 42;
    outSecondVariable = "text";
}
 
// Good
#include <tuple>
 
std::tuple<int, std::string> myFunction()
{
    return std::make_tuple(42, "text");
}
 
// auto result = myFunction();
// Use std::get<0>(result) to get the integer, std::get<1>(result) to get the std::string

When using events, prefer using lambdas (anonymous functions) instead of static functions

#include <QPushButton>
 
QPushButton *button = new QPushButton(parent);
 
// Bad
void MyObject::onClick()
{
    //...
}
 
connect(button, SIGNAL(clicked()), this, SLOT(onClick()));
 
// Good
connect(button, &QPushButton::clicked(), [this]()
{
    //...
});

When incrementing a variable, prefer using the prefix operator rather than the postfix one

It is sometimes faster, but never slower: http://stackoverflow.com/questions/24901/is-there-a-performance-difference-between-i-and-i-in-c

// Bad
i++;
 
// Good
++i;

When performing operations on containers (arrays, vectors, etc.) prefer using range-based for (= "foreach") instead of index or iterators when possible

std::vector<int> myContainer = {42, 43, 44, 45};
 
// Bad
for(std::vector<int>::iterator it = myContainer.begin(); it != myContainer.end(); ++it)
{
    //...
}
 
// Not as bad
for(int index = 0; index < myContainer.size(); ++index )
{
    //...
}
 
// Good
for(int value: myContainer)
{
    //...
}

Never forward-declare variables

// Bad
int i;
int j;
 
for(; i < 10; ++i)
{
    for(; j < 10; ++j)
    {
        //...
    }
}
 
// Good
for(int i{}; i < 10; ++i)
{
    for(int j{}; j < 10; ++j)
    {
        //...
    }
}

Never use typedef

It has been superseded by using since C++11.

// Bad
typedef int MyInteger;
 
// Good
using MyInteger = int;

Note that using using you can also set template parameters now:

using Integer3DVector = Generic3DVector<int>;

Never use #define to create constants, use constexpr instead

// Bad
#define MY_CONSTANT_VALUE 42
 
// Good
constexpr int MyConstantValue = 42; // Note the naming change here, caps should only be used for preprocessor defines

Prefer using enum classes rather that enums

// Bad
enum Color
{
    BlueColor,
    RedColor
};
 
Color myColor = BlueColor;
 
// Good
enum class Color
{
    Blue,
    Red
};
 
Color myColor = Color::Blue;

Please note that contrary to enums, enum classes cannot be implicitly converted to an integer. Use a static_cast if you need to convert from an integer to an enum class and vice-versa.

Never use C-style casts, use C++ ones

enum class Color
{
    Blue,
    Red
};
 
Color color{Color::Red};
int colorAsInteger{};
 
// Bad
colorAsInteger = (int)color;
Color otherColor = (Color)colorAsInteger;
 
// Good
colorAsInteger = static_cast<int>(color);
Color otherColor = static_cast<Color>(colorAsInteger);

When dealing with QObject-based classes you should use qobject_cast.

Header files should not depend on other #includes

To test this, just include your header in an empty source file, if it doesn't compile then you should add the missing #include directives.

Header files should be grouped by library and sorted, system headers should be at the bottom

This helps readability. Putting system headers at the bottom helps ensuring that the previous rule is enforced.

// Local headers
#include "MyClass.hpp"
 
// Library headers
#include <QDebug>
 
// System headers
#include <cmath>
#include <cstdio>

Prevent dependency contamination

Use case: you are using an “enum” or “#defines” from a system header, lets say Windows.h.

myclass.hpp
#pragma once
 
#include <Windows.h>
 
class MyClass final
{
public:
    MyClass() = default;
 
private:
    HWND m_window;
};

In this case, every file #including “myclass.hpp” will also implicitly #include Windows.h. Ouch. This dependency cannot be removed because you need to #include Windows.h to be able to use the type HWND.

Solution: here we can use the “private implementation” idiom, or “pimpl”. I consists in having all the member variables in a separate struct.

myclass_private.hpp
#pragma once
 
#include <Windows.h>
 
struct MyClass_Private
{
    HWND window;
};
myclass.hpp
#pragma once
 
#include <memory>
 
struct MyClass_Private;
 
class MyClass final
{
public:
    MyClass();
 
private:
    std::unique_ptr<MyClass_Private> m_private;
};
myclass.cpp
#include "myclass_private.hpp"
#include "myclass.hpp"
 
MyClass::MyClass():
    m_private{std::make_unique<MyClass_Private>()}
{
    // Access window by using m_private->window
}

And that's it. It is indeed an increase in complexity and has a slight impact on performance, but reduces compilation time and prevents clashes between symbols and #defines (and Windows.h creates a huge pile of mess when included).

Write small functions instead of huge ones

Never call virtual functions from a constructor

Classes and variables

You should not consider a class as a collection of variables, but rather like a service provider. That is the reason why you should put the public functions first, before the private ones. Variables should be put at the end, since they are an implementation detail.

Comments

What amount of comments should you write? Too many comments will make the code unreadable (see AutoHotKey for an example).

TODO: Add something about Doxygen and how to write compatible comments

// Bad
void stuff(int blih)
{
    int blah = 5; // We set blah to 5 here
    int bloh = std::max(blih, 8); // Here we compute the maximum value between blih and 8, and then we store the result in a variable called bloh
 
    //...  
 
    return blah; // We return the value of the variable blah of type integer. We then terminate this function and continue our merry way into our program. Yep. That's it. Bye.
}
 
// Good
// Note that this "example" has other issues: the function and the variables have meaningless names, and there is a magic value
void stuff(int blih)
{
    int blah = 5;
    // We need to compute bloh here because...
    int bloh = std::max(blih, 8);
 
    //...  
 
    return blah;
}

Qt specific

Containers

*If in doubt, use QList.

/!\ std::list is not equivalent to QList /!\

Prefer using QHash and QMultiHash over QMap and QMultiMap if you don't need the items to be sorted, their lookup time is smaller.

Inheritance

QObject-based:

  • automatic memory management (no smart pointer required)
  • constructor takes a parent QObject, defaulted to nullptr
  • Q_OBJECT macro at the beginning of the class
myobject.hpp
#pragma once
 
#include <QObject>
 
class MyObject: public QObject
{
    Q_OBJECT
 
public:
    MyObject(QObject *parent = nullptr);
    virtual ~MyObject();
}
myobject.cpp
#include "myobject.hpp"
 
MyObject::MyObject(QObject *parent):
    QObject(parent)
{
}
 
MyObject::~MyObject()
{
}

QWidget-based:

  • automatic memory management (no smart pointer required)
  • constructor takes a parent QWidget, defaulted to nullptr
  • Q_OBJECT macro at the beginning of the class
mywidget.hpp
#pragma once
 
#include <QWidget>
 
class MyWidget: public QWidget
{
    Q_OBJECT
 
public:
    MyClass(QWidget *parent = nullptr);
    virtual ~MyClass();
}
mywidget.cpp
#include "mywidget.hpp"
 
MyWidget::MyWidget(QWidget *parent):
    QWidget(parent)
{
}
 
MyWidget::~MyWidget()
{
}

Other classes:

  • memory management through smart pointers

Exceptions

Exceptions allow the developer to use various features without having to constantly check for errors. Even if you are not using them explicitly they may be triggered by the standard library or even by new. Sadly, for historical reasons, Qt does not support them. This means that if you are using a feature coming from a third party library you have to catch exceptions to prevent issues with Qt code. Note that Qt containers are exception proof however.

If you are writing non-Qt code then you really should use exceptions and more importantly, write exception-safe code. Using smart pointers is a great and easy way to do this. For Qt-based code you will have to use C-style error checking based on booleans and “getErrorString” functions. This is, for me, Qt's main drawback.

Examples

#include <string>
#include <memory>
 
// An entity semantic class
class MyExampleClass final
{
public:
    // This constructor is explicit to prevent something like this: MyExampleClass test = "some text";
    explicit MyExampleClass(const std::string &myString):
        m_myVar{52},
        m_myString{myString}
    {
    }
 
    // Entity semantic: always forbid copy & assignment
    MyExampleClass(const MyExampleClass &) = delete;
    MyExampleClass &operator=(const MyExampleClass &) = delete;
 
private:
    // Variables are always at the end, because they represent an implementation detail
    int m_myVar{42};
    std::string m_myString{"value"};
};
 
// A value semantic class
class MyVector final // Always final: a value semantic class should *never* be inherited from
{
public:
    // We want to use the default implementation (wich is faster than anything we can do)
    MyVector() = default;
 
    MyVector(const MyVector &other):
        m_x(other.m_x),
        m_y(other.m_y)
    {
    }
 
    MyVector &operator=(MyVector other)
    {
        std::swap(m_x, other.m_x);
        std::swap(m_y, other.m_y);
 
        return *this;
    }
 
private:
    // m_x and m_y are initialized with the default value for the type int: 0
    // unless the initializer list in a constructor decides otherwise
    int m_x{};
    int m_y{};
};
 
void test()
{
    // I use scopes "{}" here to limit where my variables live and are accessible
    {
        // Allocation on the stack (fast)
        MyExampleClass myExampleClass{"some text"};
        // myExampleClass is destroyed here
    }
    {
        // Allocation on the heap (slower, allows polymorphism)
        std::unique_ptr<MyExampleClass> myExampleClass{std::make_unique<MyExampleClass>("some text")};
        // Or (with auto)
        auto myExampleClass{std::make_unique<MyExampleClass>("some text")};
        // myExampleClass is destroyed here (thanks to the smart pointer)
    }
}
best_practices.txt · Last modified: 2016/03/14 12:34 by jmgr