[SOLVED] Exception safety in widget constructors?

goetz

Sydius, that's right. The base class destructor can't clean up the mess the was made in the derived class' constructor. It has to be cleaned up there. I'm not sure if the base class destructor is even called in that case. IMHO, the best way is to wrap the critical code in a try block and to clean up in the catch block if something goes wrong. The pointers should be set to null before starting the try:

@
Notepad::Notepad()
: textEdit(0),
quitButton(0)
{
QVBoxLayout *layout = 0;

try {
    textEdit = new QTextEdit(this);

quitButton = new QPushButton(tr("Quit"), this);

     connect(quitButton, SIGNAL(clicked()), this, SLOT(quit()));

    layout = new QVBoxLayout;
     layout->addWidget(textEdit);
    layout->addWidget(quitButton);

     setLayout(layout);
} catch(MyFancyException &e) {
if(textEdit)
delete textEdit;

    if(quitButton)
        delete quitButton;

    if(layout)
        delete layout;

    throw e;
}

setWindowTitle(tr("Notepad"));
}
@

ixSci

There is no need in smart pointers or try blocks here. In Qt the QObject class is responsible for cleaning up during its destruction. Also QObject is a parent class of the majority of Qt classes include the GUI ones. Although your Notepad object has not been created yet its QObject parent was. Thus you'ill have no leaks if you properly set parent at the construction time.

Sydius

[quote author="ixSci" date="1294168106"]There is no need in smart pointers or try blocks here. In Qt the QObject class is responsible for cleaning up during its destruction. Also QObject is a parent class of the majority of Qt classes include the GUI ones. Although your Notepad object has not been created yet its QObject parent was. Thus you'ill have no leaks if you properly set parent at the construction time. [/quote]

Ahh, actually you're correct. I was mistaken in believing that the base class destructor would not be called. Thank you for this clarification, it seems that Gerolf's original solution will be sufficient!

Panke

It is sufficient except that the layout (and the widgets added to it) will leak before the call to setLayout()

ixSci

Qt claims its classes are exceptions free(bold statement but almost true) that is why there is no leaks at the code presented above.

goetz

The layout does not have a parent and is therefore not caught by the QObject destructor, that's right. The layout is in the parent-child hierarchy only after setLayout() in line 17.

ixSci

Volker, exactly. But there are no possible exceptions between layout creation and its adoption by the QObject.
This ctor seems truly exception neutral to me.

goetz

In this very case, yes. But if someone does some other object creation or manipulation inbetween that can throw an exception, it can raise a problem.

Sydius

[quote author="Volker" date="1294174797"]In this very case, yes. But if someone does some other object creation or manipulation inbetween that can throw an exception, it can raise a problem.[/quote]

Having just "looked it up":http://doc.trolltech.com/latest/qvboxlayout.html, it appears that QVBoxLayout can take a parent, which should alleviate this concern?

BlackDante

[quote author="Volker" date="1294166366"]
@
Notepad::Notepad()
: textEdit(0),
quitButton(0)
{
QVBoxLayout *layout = 0;

try {
    textEdit = new QTextEdit(this);

quitButton = new QPushButton(tr("Quit"), this);

     connect(quitButton, SIGNAL(clicked()), this, SLOT(quit()));

    layout = new QVBoxLayout;
     layout->addWidget(textEdit);
    layout->addWidget(quitButton);

     setLayout(layout);
} catch(MyFancyException &e) {
if(textEdit)
delete textEdit;

    if(quitButton)
        delete quitButton;

    if(layout)
        delete layout;

    throw e;
}

setWindowTitle(tr("Notepad"));
}
@[/quote]

Hi, I have a question no related with topic but about this code. Why function "throw" is in a catch block? It is possible to do that? In this example I am little confused because I always been learnig that try block must throw the exception and in catch we catching this exception, in my university it is somethink like hard rule.

Sydius

[quote author="BlackDante" date="1294179880"]
Hi, I have a question no related with topic but about this code. Why function "throw" is in a catch block? It is possible to do that? In this example I am little confused because I always been learnig that try block must throw the exception and in catch we catching this exception, in my university it is somethink like hard rule. [/quote]

It is common practice to "rethrow" an exception from a catch block after handling it if it is not the top-level catch. This allows it to propagate to the top-most catch block, if appropriate. Having more than a top-level try/catch block is to be avoided as it's typically unnecessary (as is true in this case), and it makes little sense to rethrow from the top-most catch, which might be where you got this impression. When they do have to be nested, though, it's usually appropriate to rethrow it.

BlackDante

oh, now I understand :) thanks for explain :)

ixSci

Also you don't have to specify an object when you need to rethrow exception inside a catch block. You just need to type throw; and it will rethrow source object. Moreover you can use some languages tricks such as automated rethrow when using try\catch function block in a ctor.
Example:
@Notepad()
try
{
//blalala
throw "Exception";
}catch(...)
{
//it will rethrow automatically
}@

goetz

I will definitely have to dive deeper into that exception thingy in the future. So fare I only use that in my Java code and to catch errors from GraphicsMagick. Thanks for that tip, ixSci!

Panke

try-function-blocks are the only way to catch exceptions from the initializer list, too. I would leave a comment every time you use it. Its very simple to forget the auto-rethrow thing. My recommended reading for exception handling is "german article ":http://tinyurl.com/3yuh57x and Bjarne himself "Exception safety in TC++PL":http://www.research.att.com/~bs/3rd_safe0.html