Destructor not being called

[vieraci]

Hello,

My destructor is not being called. Can anyone tell me why ?

.h file:

class TransactionDialog : public QDialog {
    Q_OBJECT
    Q_DISABLE_COPY(TransactionDialog)
public:
//    explicit TransactionDialog(QWidget *parent = 0);
    explicit TransactionDialog(QWidget *parent = 0, int anID=0, int aType=0);
    virtual ~TransactionDialog();
 
protected:
    virtual void changeEvent(QEvent *e);
 
private:
    Ui::TransactionDialog *m_ui;
...more...

class TransactionDialog : public QDialog {
    Q_OBJECT
    Q_DISABLE_COPY(TransactionDialog)
public:
//    explicit TransactionDialog(QWidget *parent = 0);
    explicit TransactionDialog(QWidget *parent = 0, int anID=0, int aType=0);
    virtual ~TransactionDialog();

protected:
    virtual void changeEvent(QEvent *e);

private:
    Ui::TransactionDialog *m_ui;
...more...

To copy to clipboard, switch view to plain text mode 

.cpp file

TransactionDialog::~TransactionDialog()
{
    qDebug("TransactionDialog::destructor");
    delete m_ui;
    if (clientDetailsDialogCreated)
        delete clientDetailsDialog;
}

TransactionDialog::~TransactionDialog()
{
    qDebug("TransactionDialog::destructor");
    delete m_ui;
    if (clientDetailsDialogCreated)
        delete clientDetailsDialog;
}

To copy to clipboard, switch view to plain text mode 

[spirit]

it's not enough code. show us where do you create this object?

[talk2amulya]

why is your destructor virtual

[vieraci]

I don't know why it's virtual, I must have used copy/paste from other existing code and didn't notice. Anyhow it made no difference.

The object is created here:

void MainWindow::openRecord(int anID, int aType)
{
    if (aType < 4)
    {
        TransactionDialog *transactionDialog = new TransactionDialog(this, anID, aType);
        transactionDialog->show();
    }
}

void MainWindow::openRecord(int anID, int aType)
{
    if (aType < 4)
    {
        TransactionDialog *transactionDialog = new TransactionDialog(this, anID, aType);
        transactionDialog->show();
    }
}

To copy to clipboard, switch view to plain text mode 

[spirit]

is dtor of MainWindow called? looks like no.

[talk2amulya]

yeh, it wont make any difference, just that it being virtual here didnt make any sense..about your dialog, looking at the code, i m pretty sure that the destructor would be called when the MainWindow is deleted.

[vieraci]

is dtor of MainWindow called? looks like no.

Why would MainWindow's dtor have anything to do with it ?
... I see it now .....
in the ctor, it needed:

setAttribute(Qt::WA_DeleteOnClose);

setAttribute(Qt::WA_DeleteOnClose);

To copy to clipboard, switch view to plain text mode 

Thanks for the kick !

[spirit]

Why would MainWindow's dtor have anything to do with it ?
... I see it now .....
in the ctor, it needed:

setAttribute(Qt::WA_DeleteOnClose);

setAttribute(Qt::WA_DeleteOnClose);

To copy to clipboard, switch view to plain text mode 

Thanks for the kick !

because, if you remeber, when you pass a parent into ctor of objects which subclassed from QObject Qt will delete all children by itself when a parent's dtor is being called. but if you don't pass a parent in ctor of objects then you have to delete these objectcs by yourself.

[lni]

just that it being virtual here didnt make any sense..

Why? The dtor is virtual in base class already. With or without virtual makes no difference in his class. But adding virtual will remind people the method exists in vtable...

I add "virtual" keyword in derived class if the method is already defined as virtual in the based class...as I said, it is not necessary but it is a good reminder...

[wysota]

Why?

I guess the intention of the author was that no other methods are virtual so the destructor doesn't need to be virtual as well. Regardless of what happens higher in the inheritance tree.

[freeman_w]

Another way to call the dtor is to create the object NOT by a pointer-var like in your case:

...
TransactionDialog *transactionDialog = new TransactionDialog(this, anID, aType);
transactionDialog->show();
...

...
TransactionDialog *transactionDialog = new TransactionDialog(this, anID, aType);
transactionDialog->show();
...

To copy to clipboard, switch view to plain text mode 

but as an object directly, like:

...
TransactionDialog transactionDialog(this, anID, aType);
transactionDialog.show();
...

...
TransactionDialog transactionDialog(this, anID, aType);
transactionDialog.show();
...

To copy to clipboard, switch view to plain text mode 

That is only if you are not restricted in the way you have to create your object.

I had the same problem on a case with my mainwindow-object in a main.cpp file.
(problem was also solved after i added line 9 for the pointer version)
Here are the two ways that work:

int main(int argc, char *argv[])
{
    QApplication app(argc, argv);
 
    // FIRST way to reach dtors
    CPlotterMainWindow *plotmainwindow = NULL;
    plotmainwindow = new CPlotterMainWindow();
    plotmainwindow->setAttribute(Qt::WA_QuitOnClose);
    plotmainwindow->setAttribute(Qt::WA_DeleteOnClose); // needed to reach dtors
    plotmainwindow->show();
 
    // SECOND way to reach dtors
//    CPlotterMainWindow plotmainwindow;
//    plotmainwindow.setAttribute(Qt::WA_QuitOnClose);
//    plotmainwindow.show();
 
    return app.exec();
}

int main(int argc, char *argv[])
{
    QApplication app(argc, argv);

    // FIRST way to reach dtors
    CPlotterMainWindow *plotmainwindow = NULL;
    plotmainwindow = new CPlotterMainWindow();
    plotmainwindow->setAttribute(Qt::WA_QuitOnClose);
    plotmainwindow->setAttribute(Qt::WA_DeleteOnClose); // needed to reach dtors
    plotmainwindow->show();

    // SECOND way to reach dtors
//    CPlotterMainWindow plotmainwindow;
//    plotmainwindow.setAttribute(Qt::WA_QuitOnClose);
//    plotmainwindow.show();

    return app.exec();
}

To copy to clipboard, switch view to plain text mode 

Greetings

[amleto]

if you dialog is meant to be scope limited, then there is no point in using new and creating the dialog on the heap. The only reason to use the heap is for longer lifetime of instances (or occasionally forced by memory limit of stack).

Therefore you should not be using 'new' on a dialog if you are also going to use WA_DeleteOnClose (since it will necessarily be closed before it exits scope).

[freeman_w]

if you dialog is meant to be scope limited, then there is no point in using new and creating the dialog on the heap. The only reason to use the heap is for longer lifetime of instances (or occasionally forced by memory limit of stack).

Therefore you should not be using 'new' on a dialog if you are also going to use WA_DeleteOnClose (since it will necessarily be closed before it exits scope).

Hi amleto!

The "new" way is often used in many examples of qt and unless you use a modified destructor with some output for example, you will not notice that it will not be called.
You can try that in my example mentioned before. Just insert a qDebug() output in the "CPlotterMainWindow()" (or your own class that you call with "new") and you will see
that it will not be reached if that "WA_DeleteOnClose" is not there (but i think you already know that).
It was just an example there. But it still shows what it's about to show.
I didn't want to start a programming-style-discussion here
I just tried to show the way(s) how you can "reach" the destructor. And I think that this example does that.

I probably should have written that for your case:

void MainWindow::openRecord(int anID, int aType)
{
    if (aType < 4)
    {
        TransactionDialog *transactionDialog = new TransactionDialog(this, anID, aType);
        transactionDialog->show();
        transactionDialog->setAttribute(Qt::WA_QuitOnClose);
        transactionDialog->setAttribute(Qt::WA_DeleteOnClose);
    }
}

void MainWindow::openRecord(int anID, int aType)
{
    if (aType < 4)
    {
        TransactionDialog *transactionDialog = new TransactionDialog(this, anID, aType);
        transactionDialog->show();
        transactionDialog->setAttribute(Qt::WA_QuitOnClose);
        transactionDialog->setAttribute(Qt::WA_DeleteOnClose);
    }
}

To copy to clipboard, switch view to plain text mode 

Cheers