1

Usually, when writing C++ code, I would hold objects always as normal attributes, thus utilizing RAII. In QT, however, the responsibility for deleting objects can lie within the destructor of QObject. So, let's say we define some specific widget, then we have two possibilities:

1) use QT's system

class Widget1 : QWidget
{
Q_OBJECT
public:
    Widget1(QWidget* parent = nullptr);

private:
    QPushButton* myButton; // create it with "new QPushButton(this);"
};

2) use RAII

class Widget2 : public QWidget
{
Q_OBJECT
public:
    Widget2(QWidget* parent = nullptr);

private:
    QPushButton button; // normal RAII
};

Usually, I use the first method. It seems that something could work better if a parent knows its children not only by layout. But thinking about it... The real reason for this is not quite clear to me.

I know that the stack is limited. But let's say that this does not play a role here. After all, the stack is not that small.

IceFire
  • 3,719
  • 2
  • 19
  • 46
  • 3
    In your second example, `button` does not necessarily have to be allocated on the stack. class members are automatically allocated, they are destroyed when their encapsulating object is destroyed. They can be allocated on the stack or on the heap (depending on the allocation of their encapsulating object). – Mike Oct 08 '16 at 11:30
  • Please fix the question, there is no such thing as `Q_WIDGET`, it is likely that you meant the `Q_OBJECT` macro. Anyway, I think you have to remove it completely here, because it is not really needed. – Mike Oct 08 '16 at 14:38
  • Changed it now, thank you. I kept `Q_OBJECT`, though, because I want to make sure that answers will also take this into account, if it makes a difference – IceFire Oct 08 '16 at 14:51

2 Answers2

5

It seems that something could work better if a parent knows its children not only by layout.

You are right. A QObject's parent is not only used for memory management purposes, this answer sums some of its other usages. The most important ones here are the ones of QWidget's (since you are concerned about adding member QWidget's), so if you use the second approach the way you are writing it, here are some of the issues you might get:

  • Suppose you are instantiating Widget1 and displaying it in your main function like this:

    Widget1 w;
    w.show();
    

    This will display an empty widget without the button inside it. As opposed to the behavior when button is a child of the Widget1 object, where calling show() displays the widget parent with all of its children inside.

    Similar issue happens when using setEnabled(), setLayoutDirection(), ...

  • button.pos() won't return coordinates relative to Widget1, in fact the button is not even displayed inside it. Same issue when using move().

  • The event system might not work as expected. So, if the member widget does not handle some mouse/keyboard event, the event won't propagate to the parent widget (since there is no parent specified).

But the second approach can be written to utilize the parent relationship with RAII, so that above issues are avoided:

class Widget2 : public QWidget
{
public:
    explicit Widget2(QWidget* parent = nullptr):QWidget(parent){}

private:
    QPushButton button{this}; //C++11 member initializer list
};

Or, in pre-C++11:

class Widget2 : public QWidget
{
public:
    //initialize button in constructor, button's parent is set to this
    explicit Widget2(QWidget* parent = Q_NULLPTR):QWidget(parent), button(this){}

private:
    QPushButton button;
};

This way there aren't any differences to the Qt Framework between both approaches. In fact, using the second approach avoids dynamic allocation when unnecessary (Note that, this might have slightly better performance, if the allocation is executed very frequently in some nested loop for example. But, for most applications, performance is not really an issue here). So, you might be writing your widgets like this:

class Widget : public QWidget
{
public:
    explicit Widget(QWidget* parent = nullptr):QWidget(parent){
        //add widgets to the layout
        layout.addWidget(&button);
        layout.addWidget(&lineEdit);
        layout.addWidget(&label);
    }
    ~Widget(){}

private:
    //widget's layout as a child (this will set the layout on the widget)
    QVBoxLayout layout{this};
    //ui items, no need to set the parent here
    //since this is done automatically in QLayout::addWidget calls in the constructor
    QPushButton button{"click here"};
    QLineEdit lineEdit;
    QLabel label{"this is a sample widget"};
};

This is perfectly fine, one might say that those child widgets/objects will be destroyed twice, first when they are out of scope and a second time when their parent is destroyed, making the approach unsafe. This is not an issue as once a child object is destroyed it removes itself from its parent's children list, see docs. So, each object will be destroyed exactly once.

Mike
  • 7,220
  • 1
  • 23
  • 39
  • "can have better performance because it avoids dynamic allocation when unnecessary" - when something happens once twice in a row (no loops, like in this case), worrying about heap performance is completely pointless. – Marek R Oct 09 '16 at 07:39
  • @MarekR, Of course, I don't mean that this can cause a real performance bottleneck. So, unless it is something that is executed too repeatedly, it can make some very little difference to avoid dynamic allocation. So, this is not really a kind of performance issue in most of programs. One should worry first about the algorithms being used, and run the program through a profiler to identify its real bottleneck. But, my point is, why would one use dynamic allocation if it is not really needed here? Thanks for the comment, I've edited the question to clarify the point. – Mike Oct 09 '16 at 12:32
  • 2
    @MarekR It is clear premature pessimization: not only do you have to allocate dynamically once more, but you reference the object via an extra layer of indirection, wasting precious cache. – Kuba hasn't forgotten Monica Oct 10 '16 at 18:31
  • did you meant "premature optimization" or you are one of this developers who spent days to speed up code by 10% which is run once twice per 10 minutes (or application start)? – Marek R Oct 11 '16 at 08:13
-1

First of all you are asking about basics of C++ object oriented programing. Standard rules of object destruction applies.

Secondly you are wrong. Stack here is not involved. You widget is placed as a class field so widget became part of your class. I will be located on stack only if you instantiate Widget1 on stack.

Size of stack doesn't matter here. Qt uses d-pointer code pattern in all QObjects. This means that Qt classes have usually small constant size (I didn't check but most probably on 32 bit systems it will be 8 bytes: pointer to virtual table and d-pointer). Even if you instantiate object of your class on stack data alignment done by compiler can reduce difference of stack memory usage for both cases to zero (size of classes is usually rounded up by compiler to paragraph size - 16 bytes), so in your example there no reason to worry about that.

If you place a button as a part of your Widget1 there is no big difference from you application logic point of view (you've wrote "parent knows its children not only by layout" this means that Widget1 is always a parent to button).

You should use pattern with a pointer to keep code clean. In well maintained project it is important to keep header files as clean as possible. This means that you should use forward declaration and avoid unnecessary includes in header file, for example:

#include <QWidget.h> // inheriting from this class os this include is needed

QT_FORWARD_DECLARE_CLASS(QPushButton) // don't have to include "QPushButton.h" in this header file

class Widget1 : QWidget
{
Q_OBJECT
public:
    Widget1(QWidget* parent = nullptr);

private:
    QPushButton* myButton;
};

Forward declarations are used big projects since it reduces a building time. If project is big this can be very important. If you would use widget as a direct value you have include its header file "QPushButton.h".

Marek R
  • 23,155
  • 5
  • 37
  • 107