W.K.S
W.K.S

Reputation: 10095

Confusion about usage of 'new' for UI Widgets in QMainWindow constructor

My coding practice using Qt can best be described as follows:

  1. If the Widget is going to be actively used (e.g. A QLineEdit which provides text), I declare it in the header file and then initialise it in MainWindow.cpp. e.g. TextEditor.h:

class TextEditor { //other code private: QLineEdit edtFind; };

2.. If a widget is not going to be used (e.g. QLabel, QWidget), or it's part of a signal slot system (e.g. QPushButton), I declare and inialise it inside constructor using new.

-e.g.

TextEditor::TextEditor()
{
   //other code
   QWidget* searchPanel = new QWidget();
   edtFind = new QLineEdit("Enter Search Term");
   QPushButton* findButton = new QPushButton("Find");
   connect(findButton,SIGNAL(pressed()),this,SLOT(find()));

   ui->statusbar->addPermanentWidget(searchPanel);
}

My question is, am I using an efficient approach in point 2? Would it be better to not allocate memory from the heap?

Thanks.

Upvotes: 1

Views: 310

Answers (2)

danimo
danimo

Reputation: 574

While good advise for C++ in general, answer 1 is actually wrong for a big part in Qt: QObject (and with it all widgets, since QWidget derives from QObject). Rule there is to always allocate QObjects on the heap if they have a parent, because QObject features a parent-based garbage collection (when the topmost QObject-parent gets deleted, it will ask all its children to delete themselves recursively). The application may try to delete an object on the stack, which leads to a crash.

Note that some operations in Qt implicitly add or change the parent of a QObject as a side-effect (reparenting), such as adding a widget to a layout. However, this is usually documented in the API documentation. Since reparenting is very common with QWidgets, you should never put them on the stack. Other QObject-derived classes are safer, consult the API documentation in case of doubt.

Upvotes: 1

Nikola Smiljanić
Nikola Smiljanić

Reputation: 26873

Your approach is not efficient. You should use heap allocated objects when you actually need them:

  • objects that have a longer lifetime
  • using a forward declaration in order to avoid including a header file
  • holding a reference to an object created elsewhere

Your approach is more complicated without any visible benefit. Heap is known to be slow, and allocating a large number of small objects is known to fragment it (this might not make a difference in your app but it's still a bad practice).

Upvotes: 2

Related Questions