Are default memory allocations in class declaration considered a good practice?

user_14971567 0
Viewed 102

I have a class the members of which are allocated on heap

// client.h

#include <QString>
#include <QTcpSocket>
#include <QTextEdit>
#include <QLineEdit>
#include <QWidget>

class Client : public QWidget
{
    Q_OBJECT

public:
    Client(const QString &computerName, int portNumber,
           QWidget *parent = nullptr);

    ~Client() = default;

// ...

private:
    QTcpSocket *_tcpSocket = new QTcpSocket(this);
    QTextEdit *_textInfo = new QTextEdit;
    QLineEdit *_textInput = new QLineEdit;
    quint16 _nextBlockSize = 0;
};

// client.cpp

Client::Client(const QString &computerName, int portNumber, QWidget *parent)
    : QWidget(parent)
{
    // ...
}

I never seen default initialization done this way - whether it was done in the ctor or at the best case in the initialization section of it (e.g. _tcpSocket(new QTcpSocket(this))). I can think of multiple reasons against it:

  • because we invoke the ctors of the objects we allocate using new, we can't use forward declaration now
  • seeing these allocations in the class definition might suggest the user that the class has a non-trivial dtor and the objects must be freed when in fact they don't, because Qt will manage them itself
  • seeing this in _tcpSocket = new QTcpSocket(this) is just strange to me

So what's the best way to default assign objects on heap?

c++
3 Answers

A default member initialiser that allocates memory is not a bad practice.

However, not deleting such dynamic allocations in the destructor is a bad practice. Owning bare pointers in the first place are a bad practice. Furthermore, unnecessary dynamic allocation is a bad practice. But that's apparently a problem with the design of Qt.

we can't use forward declaration now

Sure you can. But I think that you mean that a forward declaration won't be sufficient. That's a good point. If you need to avoid defining those member types, then default member initialisers are not an option in such case.

user_20793030

I can think of multiple reasons against it:

  • because we invoke the ctors of the objects we allocate using new, we can't use forward declaration now It is not bad practice, but I would not do it. Why? It introduces dependencies in header files (blocks forward declarations), which forces includes what slows down build time.

This is very big issue. For single source build time difference is not noticeable. If project is big and has hundreds of files it has great impact on build time.

It is even more important to forward declare classes of the project, since small change in one class will lead to cascade of rebuilds over whole project if forward declaration is not used.

I do not see gain to have this done in header and losing forward declaration IMO is big pain, so I would not do it and try force this as a rule in project.

user_13874380

Direct initialisation of class members is available since C++11.

It has some benefits if you have more then one Constructor. All members are initialized with the same values and you don't have to do it in each constructor again (which is a potential source for errors, if you change the init value some time). And its a good way to properly initalize your members and follow the rule of 0. The initialisation order is exactly as the members are ordered in the class.

In your case, there is no error handling (for new, e.g. "out of memory"), this is a disadvantage. But if you init it in the constructor like

Client::Client(const QString &computerName, int portNumber, QWidget *parent)
: QWidget(parent)
, _tcpSocket = new QTcpSocket(this)
{}

you have no error handling, too.

I personally prefer the direct initialisation, because in combination with the rule of 0 the classes are a lot cleaner.

But you are correct with the forward declaration topic, this is not possbilbe with this approach.

user_53493730
Related