Inner classes in Qt

Question

I am using Qt5 with the VS2013 compiler. I am trying to nest two classes CUDial and DUDial in the same outer class UDial.

UDial is a QDialog type class. I would like CUDial and DUDial to be both of QWidget type. It for use with a QTabWidget variable in the outer class.

So, I write the code as follow:

class UDial : public QDialog
{
    Q_OBJECT

    class CUDial : public QWidget
    {
        Q_OBJECT

        // Some variables

     public:

        CUDial(QWidget *parent = 0);
    }wid1;

   class DUDial : public QWidget
   {
        Q_OBJECT

        // Some variables

    public:

        DUDial(QWidget *parent = 0);
   }wid2;

   QTabWidget *tab;
   QDialogButtonBox *box;
   QVBoxLayout *vlay;

public:

   UDial(QWidget *parent = 0);
};

After I implemented the code, I tried to compiled and got the following C2664 error:

error: C2664: 'int QTabWidget::addTab(QWidget *,const QIcon &,const QString &)' : cannot convert argument 1 from 'UDial::CUDial' to 'QWidget *'

No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called

I guess that it is a problem with the QWidget inheritance of my nested classes.

Is there a way I can solve that issue?

Answer 1

There are two problems:

1. A trivial one: the lack of object dereference elsewhere in your code - a typo.

2. The meta object compiler (moc) does not support nested classes. Your code will compile, but it won't pass the moc step, with the following error:

   Error: Meta object features not supported for nested classes

I do commend you for not prematurely pessimizing the storage of wid1 and wid2. You stored them by value - as you should. Ideally, though, you should store all such objects by value - not only the two sub-dials. It will also make things easier to read to decouple the instantiation of the inner classes from their declaration.

So, the sub-dial types need to be moved out of the class. You can put them in a namespace so that they don't pollute the global one.

namespace UDial_ {
class CUDial : public QWidget {
   Q_OBJECT
public:
   CUDial(QWidget *parent = 0) : QWidget(parent) {}
};

class DUDial : public QWidget
{
   Q_OBJECT
public:
   DUDial(QWidget *parent = 0) : QWidget(parent) {}
};
}

Secondly, once you do store objects by value, the children must be declared after the parents. This is because the compiler will generate code to destruct them in the reverse order of declaration. The child QObjects must be destroyed before the parents, otherwise the parents will, incorrectly, attempt to delete them.

I don't know of any way to force the compiler to enforce this without changing Qt source code, but you can at least indicate your intent by using the C++11 value initialization.

class UDial : public QDialog
{
   Q_OBJECT
   QVBoxLayout vlay { this }
   QTabWidget tab;
   // We make it explicit that both sub-dials are children of tab and must not
   // be declared / initialized before it!
   UDial_::CUDial wid1 { &tab };
   UDial_::DUDial wid2 { &tab };
   QDialogButtonBox box;

public:
   UDial(QWidget *parent = 0);
};

Unfortunately, the compiler will at best emit a warning if we do the wrong thing, although any decent static analyzer will loudly complain about it, of course.

class UDial : public QDialog
{
   Q_OBJECT
   QVBoxLayout vlay { this };
   UDial_::CUDial wid1 { &tab };
   UDial_::DUDial wid2 { &tab };
   QTabWidget tab; // WRONG, comes after the use above
   QDialogButtonBox box;

public:
   UDial(QWidget *parent = 0);
};

This code will, in most cases, crash upon startup, but that's not a crash we can depend on - say, as if we dereferenced a nullptr. But beware: the compiler is free to optimize out any dereferences of a nullptr!

The constructor will then look similar to:

UDial::UDial(QWidget * parent) : 
  QDialog(parent),
  box(QDialogButtonBox::Ok | QDialogButtonBox::Cancel)
{
  vlay.addWidget(&tab);
  vlay.addWidget(&box);
  tab.addTab(&wid1, "Dial 1");
  tab.addTab(&wid2, "Dial 2");
}

If you're using an older compiler that doesn't support C++11 value initialization, you cannot use the brace initialization syntax, and you are forced to declare your intent in the constructor - where it is more likely to be ignored by an unwitting maintainer:

UDial::UDial(QWidget * parent) : 
  QDialog(parent),
  vlay(this),
  wid1(&tab), wid2(&tab),
  box(QDialogButtonBox::Ok | QDialogButtonBox::Cancel)
{ ...

---

BartoszKP has a valid argument that since the checking of this cannot be reliably relegated to compile time, there is some merit to explicitly allocating all sub-objects on the heap, and letting Qt deal with proper destruction order at run-time. While I personally haven't run into any problems related to that, I'm very diligent and in a privileged position of having me and myself to maintain my code base. In larger projects that are not managed with proper process documentation in place to point out such procedural details, this approach may lead to errors. Such errors are, IMHO, more of an indication of a procedural issue in the development process than the validity of the approach. It then becomes a premature pessimization to work around development process deficiencies. It's up to you to decide whether that's a valid tradeoff. I personally cringe when I see the result of new assigned to a raw pointer - it tends to desensitize one to such incorrect uses where you don't have a nice memory-managing class like QObject.

Answer 2

You are not showing the code which is responsible for the error. But from the error message it looks like you are doing something like this:

sometabwidget->addTab(some_CUDial_instance, ...);

when you should simply be passing a pointer to it:

sometabwidget->addTab(&some_CUDial_instance, ...);

Answer 3

After some discussions in the comments I admit the approach I encourage here is a bit opinion based, so I strongly advise to read all answers under this question. However, please do note the paragraph with statements in bold in this answer.

---

In general, following Qt's suggestion from the documentation:

QObjects organize themselves in object trees. When you create a QObject with another object as parent, it's added to the parent's children() list, and is deleted when the parent is. It turns out that this approach fits the needs of GUI objects very well. For example, a QShortcut (keyboard shortcut) is a child of the relevant window, so when the user closes that window, the shortcut is deleted too.

you should not hold instances of QWidget types by value, because when adding them to QTabWidget it will become their parent and will attempt to delete them on deconstruction.

The problem lies in the fact that you are passing an object by value (wid1) to a function which requires a pointer (QTabWidget::addTab). Usually you can just change it to &wid1 but it can cause problems in the aforementioned context of Qt's memory management.

In your particular case, it will work fine, because the destruction will start in your class, deleting its members, and they will correctly unregister from QTabWidget, so it won't attempt to delete them again later.

This however can lead to an inflexible code structure - as explained in the linked page of Qt's documentation, where simple reordering of object construction leads to a crash.

Also, you will need to manually reorder the members inside your class, to follow their dependencies as your code evolves. This seems silly, if you don't have to do it: Qt will do just fine managing the life-time of your widgets, and you don't have to worry about it.

Thus, the safest solution is to have a pointer CUDial* wid1, allocate it with new and let QTabWidget manage its life-time.

Answer 4

I found this via search for "qt meta object features not supported for nested classes" (which would have been the next error after the dereference issue, as Kuba's answer explains).

One way to work around the Qt object "nested classes" issue is to forward declare the nested class in the enclosing class. Then declare the nested class outside the encloser. E.g.:

class UDial : public QDialog
{
    Q_OBJECT
    class CUDial;   // forward declare (doesn't have to be private)
    CUDial *wid1;  // would need to be a pointer if you need to hold a reference to it (which maybe you don't)

  public:
    UDial(QWidget *parent = 0);
};

class UDial::CUDial : public QWidget
{
    Q_OBJECT
    CUDial(QWidget *parent = 0);
    void someOtherFunction();
    friend class UDial;  // could also make the constuctor/etc public
};

In source (.cpp), CUDial (and any member functions) would be defined like this:

  UDial::CUDial::CUDial(QWidget *parent) : QWidget(parent) { ... }
  void UDial::CUDial::someOtherFunction() { ... }