Object destruction in Qt dialog

I am new to Qt and am trying to understand object destruction and where I need to explicitly destroy objects using delete.

class MainDialog: public QWidget {
        Q_OBJECT
    public: MainDialog(QWidget *parent=0);
    private:
        QPushButton *m_newClick;
          QPushButton *m_editClick;
         MainContact *m_contact;
    }

MainDialog::MainDialog(QWidget *parent):QWidget(parent) {
    QWidget *container= new QWidget();

    QPushButton *m_newClick =new QPushButton("New", container);
    QPushButton *m_editClick= new QPushButton("Edit", container);

    QLineEdit *m_text=new QLineEdit(container);

    QVBoxLayout *vlayout=new QVBoxLayout();
    QHBoxLayout *hlayout=new QHBoxLayout();
    hlayout->addWidget(m_newClick);
    hlayout->addWidget(m_editClick);
    vlayout->addLayout(hlayout);

    connect(m_viewClick,SIGNAL(clicked()),this,SLOT(viewClick()) );
    connect(m_newClick,SIGNAL(clicked()),this,SLOT(newClick()) );

    setWindowTitle("Welcome");

    setLayout(vlayout);

}

My queries:

• Will the Pushbuttons and lineEdits get destroyed once container is destroyed?

• Do I need to destroy m_contact separately as it is not part of the container?

I would like a small explanation on the parent child object destruction as well.

I see a number of issues with your code, but let me answer your questions first:

1. Will the Pushbuttons and lineEdits get destroyed once 'container' is destroyed?

Yes, the pushbuttons and lineedits get destroyed when container gets destroyed

1. Do I need to destroy m_contact separately?

I do not see where m_contact is created. In the above code you just declare a pointer to a MainContact object. Nothing is created/allocated, so nothing needs to be destroyed.

Recommended reading is the detailed description of the QObject documentation:


QObjects organize themselves in object trees. When you create a QObject with another object as parent, the object will automatically add itself to the parent's children() list. The parent takes ownership of the object; i.e., it will automatically delete its children in its destructor.

I have the impression that you do not have a clear idea what you need to do with all the pointers/objects etc. If my further comments sound like spoonfeeding, then I'm sorry for this. But I think a step-by-step discussion of your code will help you (my comments always after the code):

class MainDialog: public QWidget {

here you declare your class as a subclass of QWidget. this means your class will be a QWidget and therefore a QObject too.

Q_OBJECT
    public: MainDialog(QWidget *parent=0);
    private:
        QPushButton *m_newClick;
          QPushButton *m_editClick;
         MainContact *m_contact;

you declare three pointers. No object will be created by this. This is just your handle to have access to the objects later.

}

MainDialog::MainDialog(QWidget *parent):QWidget(parent) {

with QWidget(parent) you initialize the QWidget component of your class MainDialog

QWidget *container= new QWidget();

here you create a local pointer to a QWidget and create a new QWidget object without specifying a parent of this new object. Local means that this variable is only valid within the scope of this function. To be exact: the scope is between the curly brackets {}. The variable will be destroyed when the program flow leaves the scope.

QPushButton *m_newClick =new QPushButton("New", container);
    QPushButton *m_editClick= new QPushButton("Edit", container);

here you create two new local pointers to QPushButton objects which (!!!) hiding the pointers that you have created as private members of your class. Not all compilers will warn you about this. Your code should be just this: m_newClick =new QPushButton("New", container);

QLineEdit *m_text=new QLineEdit(container);

as above m_text is a local pointer and will point to a newly created QLineEdit object.

QVBoxLayout *vlayout=new QVBoxLayout();
    QHBoxLayout *hlayout=new QHBoxLayout();
    hlayout->addWidget(m_newClick);
    hlayout->addWidget(m_editClick);
    vlayout->addLayout(hlayout);

This bit is correct. You are using local pointers to the layouts which is absolutely fine; you don't need them later on. One important point here: the ownership is transferred to the layout. That means, the container object is no longer parent of the pushbuttons.

You could avoid the use of local varables by writing it like this: hlayout->addWidget(new QPushButton("New")); but then you have no handle to connect with.

connect(m_viewClick,SIGNAL(clicked()),this,SLOT(viewClick()) );

the variable m_viewClick is not defined. This is probably just a typo here on CR. But the slot viewClick() is also not defined in the class above.

connect(m_newClick,SIGNAL(clicked()),this,SLOT(newClick()) );

same here: newClick() is not defined.

setWindowTitle("Welcome");

    setLayout(vlayout);

here you set the layout that contains your previously constructed objects to this MainDialog object. MainDialog becomes the new owner of vlayout and with it all the pushbuttons and lineedits.

Perhaps you also need to call show().

}

here we are at the end of the scope of MainDialog constructor function. At this point the following variables will be destroyed: container m_newClick m_editClick m_text vlayout hlayout. These are just the pointers. Since the ownership of all the objects has been transferred, no object will be destroyed. This results in a "leak" of the container and m_text object.

Summary of the problems:

• the container object is useless

• the three private variables m_newClick m_editClick and m_contact are not used.

• the local variables m_newClick and m_editClick hide private member variables. This is a perfect example why prefixes on member variables are bad.

• don't connect non-existing functions.

The following is an example with the above suggestions implemented:

```
class MainDialog : public QWidget
{
Q_OBJECT
public: MainDialog(QWidget *parent=0);
public slots:
void newClick();
void editClick();
void endEdit();
}

MainDialog::MainDialog(QWidget *parent)
: QWi

StackExchange Code Review Q#105241, answer score: 4