Extending a common base: Diamond inheritance vs. QObject

Question

I think I've run into a kind of diamond inheritance problem here.

Qt provides a couple of spin boxes, for integer values, for doubles and also for dates/times. They all derive from QAbstractSpinBox:

#include <QtWidgets/QSpinBox>
class QSpinBox:
    public QAbstractSpinBox {

};

#include <QtWidgets/QDoubleSpinBox>
class QDoubleSpinBox:
    public QAbstractSpinBox {

};

Now I'd like to add some functionality common to all spin boxes, in this concrete example, a button to revert the spin box to its minimum (hence specialValueText). So I also derived from QAbstractSpinBox and came up with something like this:

class AbstractRevertibleSpinBox:
    public QAbstractSpinBox {

    public:
        RevertibleSpinBox() {
            /* Use abstract base: */
            QAction *revertAction = new QAction(this);
            QAbstractSpinBox::lineEdit()->addAction(
                revertAction, QLineEdit::TrailingAction);
            /* ... */
        }

    public slots:
        virtual void revert()  = 0;
}

This containts the pure revert() that should implement how to revert the different spin boxes. For example, using setValue(double) for the QDoubleSpinBox or setDate(QDate) for the QDateEdit. And then I went the obvious way and derived the appropriate classes for all the spin boxes I needed, like these:

class RevertibleSpinBox:
    public QSpinBox,
    public AbstractRevertibleSpinBox {

    protected:
        void revert() {
            /* Revert 'int' */
            setValue(0);
        }
};

class RevertibleDoubleSpinBox:
    public QDoubleSpinBox,
    public AbstractRevertibleSpinBox {

    protected:
        void revert() {
            /* Revert 'double' */
            setValue(0.0);
        }
};

This obviously does not work as anything in QAbstractSpinBox is ambiguous now. I thought I could resolve it using virtual inheritance and this would work if, e.g. QDoubleSpinBox would virtually derive from its own QAbstractSpinBox. But it doesn't. Also, it would fail with QObject because Qt seems to do a lot of static_cast upwarts there, which also does not work with virtual inheritance. I also thought of resolving it by making the AbstractRevertibleSpinBox a template class that is passed the distinct spin box type as a template class parameter. The construction would then look like this:

template<class Base>
class AbstractRevertibleSpinBox:
    public Base {};

class RevertibleSpinBox:
    public AbstractRevertibleSpinBox<SpinBox> { };

This would work, however Qt's moc is very unhappy about template classes. So for example, I cannot connect any signals and slots from within the template class. At least not using the conventional string based SIGNAL()/SLOT() syntax.

Is there any other reasonably elegant way to overcome this problem..?

Cystine

Good question, would upvote if I had not reached the daily vote limit... Why do you not use the decorator design pattern? This looks like a great use case for that. – Osteopathy

If no one else works out the nitty-gritty details for you until the evening, I will try to. Finally, a really good question on Stack Overflow. ;-) – Osteopathy

The problem is that I need protected methods from the QAbstractSpinBoxBase, like lineEdit(). I could solve this by creating a custom base that derives from QAbstractSpinBoxBase and offers a public API to the protected methods. Also, I wish I could avoid boilerplate-copying all the inferior spin box' methods into the wrapping class :-/ Thank you, this was my first question here :-) – Cystine

To clarify (a thing not that easy to me..), my actual goal was to keep the final derived class still be QSpinBox/QDoubleSpinBox/.. itself. So there would be no need to write wrapping methods to an inferior object, like when using a compositing pattern. – Cystine

Looking forward to learning more about the pattern. Thank you! – Cystine

Please do not edit your question with others' answer. This is not the way how questions work on Stack Overflow, especially without giving credit to others who solved the issue for you. – Osteopathy

I was going to edit the question to give credit to all who contributed so far. Unfortunately, the issue has not been resolved so far. (And by the way, below you told me not to post a semi-question as answer. Ok. Mohit Jain as well as some posts on meta suggest to amend the question with an update. Ok. You tell me not to do so. So where for good to amend the source excerpt?! I bet using some paste bin would reveal criticism as well.) – Cystine

Answer 1

As indicated right upfront in my comment, I think this is a clear case for the Decorator Pattern if you want an easily extensible feature system, otherwise just inherit from QObject rather than from the base "interface" with pretty much the same code.

I will start with the IMHO worse approaches, supplied in the other answers given:

• Subclassing each spin box

This is obviously tiresome and even more important, you will not be able to support any QSpinBox subclass with those as you would always need to create a new subclass for each addition. It is simply an unflexible approach.

• Have a parent widget containing the button and spin box

This looks like an unnecessary coupling of two different things, and so you would not be able to reuse spin boxes easily should you trigger them any other way later than through buttons. I think the two concepts should remain distinct and separately managed.

Furthermore, dynamic_casting is wrong as suggested as you should use qobject_cast if any.

Let us have a closer look at the decorator approach:

This is not yet the solution for your case, but it shows pretty well how features can be added (i.e. "decorated") into existing hierarchies. To get a bit more concrete about your use case, let us see what would be what in your particular scenario:

• Component: QAbstractSpinBox

• Concrete components

  • QSpinBox
  • QDoubleSpinBox
  • QDateTimeEdit
    • QDateEdit
    • QTimeEdit

• Decorator: AbstractSpinBoxDecorator (this step can be left out in your case)

• Concrete Decorator: RevertibleSpinBoxDecorator

Let us get our hands dirty with implementing this design:

main.cpp

#include <QAbstractSpinBox>
#include <QSpinBox>
#include <QDoubleSpinBox>
#include <QDateTimeEdit>
#include <QDateEdit>
#include <QTimeEdit>
#include <QPushButton>
#include <QApplication>
#include <QMainWindow>
#include <QHBoxLayout>
#include <QWidget>
#include <QShowEvent>

class RevertibleSpinBoxDecorator : public QAbstractSpinBox
{
    Q_OBJECT
public:
    explicit RevertibleSpinBoxDecorator(QAbstractSpinBox *abstractSpinBox, QAbstractSpinBox *parent = Q_NULLPTR)
        : QAbstractSpinBox(parent)
        , m_abstractSpinBox(abstractSpinBox)
    {
    }

public slots:
    void revert(bool)
    {
        QSpinBox *spinBox = qobject_cast<QSpinBox*>(m_abstractSpinBox);
        if (spinBox) {
            spinBox->setValue(spinBox->minimum());
            return;
        }

        QDoubleSpinBox *doubleSpinBox = qobject_cast<QDoubleSpinBox*>(m_abstractSpinBox);
        if (doubleSpinBox) {
            doubleSpinBox->setValue(doubleSpinBox->minimum());
            return;
        }

        QDateEdit *dateEdit = qobject_cast<QDateEdit*>(m_abstractSpinBox);
        if (dateEdit) {
            dateEdit->setDate(dateEdit->minimumDate());
            return;
        }

        QTimeEdit *timeEdit = qobject_cast<QTimeEdit*>(m_abstractSpinBox);
        if (timeEdit) {
            timeEdit->setTime(timeEdit->minimumTime());
            return;
        }

        QDateTimeEdit *dateTimeEdit = qobject_cast<QDateTimeEdit*>(m_abstractSpinBox);
        if (dateTimeEdit) {
            dateTimeEdit->setDateTime(dateTimeEdit->minimumDateTime());
            return;
        }

        Q_ASSERT_X(false, "decorator", "concrete component unimplemented");
    }

protected:
    void showEvent(QShowEvent *event) Q_DECL_OVERRIDE
    {
        m_abstractSpinBox->show();
        event->ignore();
        hide();
    }

private:
     QAbstractSpinBox *m_abstractSpinBox;
};

class MainWindow : public QMainWindow
{
    Q_OBJECT
    public:
        explicit MainWindow(QWidget *parent = Q_NULLPTR) : QMainWindow(parent)
        {
            connect(pushButton, &QPushButton::clicked, revertibleSpinBoxDecorator, &RevertibleSpinBoxDecorator::revert);
            QHBoxLayout *layout = new QHBoxLayout(centralWidget);
            layout->addWidget(revertibleSpinBoxDecorator);
            layout->addWidget(pushButton);
            setCentralWidget(centralWidget);
        }

    private:
        QWidget *centralWidget{new QWidget(this)};
        QDoubleSpinBox *doubleSpinBox{new QDoubleSpinBox(this)};
        RevertibleSpinBoxDecorator *revertibleSpinBoxDecorator{new RevertibleSpinBoxDecorator(doubleSpinBox)};
        QPushButton *pushButton{new QPushButton(this)};
};

#include "main.moc"

int main(int argc, char **argv)
{
    QApplication application(argc, argv);
    MainWindow mainWindow;
    mainWindow.show();
    return application.exec();
}

If you want to get rid of the QAbstractSpinBox inheritance, you will need to apply a bit more glue and IMHO for not much gain, while losing the flexibility. You would start off with something like this:

Non-Decorator

#include <QAbstractSpinBox>
#include <QSpinBox>
#include <QDoubleSpinBox>
#include <QDateTimeEdit>
#include <QDateEdit>
#include <QTimeEdit>
#include <QPushButton>
#include <QApplication>
#include <QMainWindow>
#include <QHBoxLayout>
#include <QWidget>
#include <QShowEvent>

class RevertibleSpinBoxDecorator : public QObject
{
    Q_OBJECT
public:
    explicit RevertibleSpinBoxDecorator(QAbstractSpinBox *abstractSpinBox, QObject *parent = Q_NULLPTR)
        : QObject(parent)
        , m_abstractSpinBox(abstractSpinBox)
    {
    }

public slots:
    void revert(bool)
    {
        QSpinBox *spinBox = qobject_cast<QSpinBox*>(m_abstractSpinBox);
        if (spinBox) {
            spinBox->setValue(spinBox->minimum());
            return;
        }

        QDoubleSpinBox *doubleSpinBox = qobject_cast<QDoubleSpinBox*>(m_abstractSpinBox);
        if (doubleSpinBox) {
            doubleSpinBox->setValue(doubleSpinBox->minimum());
            return;
        }

        QDateEdit *dateEdit = qobject_cast<QDateEdit*>(m_abstractSpinBox);
        if (dateEdit) {
            dateEdit->setDate(dateEdit->minimumDate());
            return;
        }

        QTimeEdit *timeEdit = qobject_cast<QTimeEdit*>(m_abstractSpinBox);
        if (timeEdit) {
            timeEdit->setTime(timeEdit->minimumTime());
            return;
        }

        QDateTimeEdit *dateTimeEdit = qobject_cast<QDateTimeEdit*>(m_abstractSpinBox);
        if (dateTimeEdit) {
            dateTimeEdit->setDateTime(dateTimeEdit->minimumDateTime());
            return;
        }

        Q_ASSERT_X(false, "strategy", "strategy not implemented");
    }

private:
     QAbstractSpinBox *m_abstractSpinBox;
};

class MainWindow : public QMainWindow
{
    Q_OBJECT
    public:
        explicit MainWindow(QWidget *parent = Q_NULLPTR) : QMainWindow(parent)
        {
            connect(pushButton, &QPushButton::clicked, revertibleSpinBoxDecorator, &RevertibleSpinBoxDecorator::revert);
            QHBoxLayout *layout = new QHBoxLayout(centralWidget);
            layout->addWidget(doubleSpinBox);
            layout->addWidget(pushButton);
            setCentralWidget(centralWidget);
        }

    private:
        QWidget *centralWidget{new QWidget(this)};
        QDoubleSpinBox *doubleSpinBox{new QDoubleSpinBox(this)};
        RevertibleSpinBoxDecorator *revertibleSpinBoxDecorator{new RevertibleSpinBoxDecorator(doubleSpinBox)};
        QPushButton *pushButton{new QPushButton(this)};
};

#include "main.moc"

int main(int argc, char **argv)
{
    QApplication application(argc, argv);
    MainWindow mainWindow;
    mainWindow.show();
    return application.exec();
}

main.pro

TEMPLATE = app
TARGET = main
QT += widgets
CONIG += c++11
SOURCES += main.cpp

Build and Run

qmake && make && ./main

Lloyd

Answer 2

I don't think I would use inheritance here at all; instead I'd use composition: create an unrelated class (e.g. derived from QWidget) that uses a QBoxLayout (or similar) to arrange the spin-box and the button as child widgets. The appropriate spin-box object-pointer could be passed to the class's constructor, which would also do the necessary connect() commands to forward the various signals back and forth. You'd probably want to redeclare equivalents for the QAbstractSpinBox class's slots/signals/methods in your class, of course, but the upside is that it would work with any of the QAbstractSpinBox sub-classes.

(As for getting revert() to do the right thing, the easiest approach might be just some ugly special-case logic using dynamic_cast<> -- since there are only three QAbstractSpinBox subclasses you need to support, that will be manageable, and at least that way the ugliness is hidden inside a private method body rather than being exposed to the class's users)

Semipostal

Thank you Jeremy. The downside of your approach, however, would be that I would have to 'boilerplate' all of the inferior spin boxes' methods that are needed from outside and that are not available in the QAbstractSpinBox base. Because they depend on the type of the actual spin box (int/double/QDate/...). I hope I am mistaken here, tho :-) – Cystine

True... I suppose you could use the QVariant class as your catch-all data type if you wanted to avoid too much duplication. – Semipostal

Answer 3

One option to consider might be to simply create classes that derive from each of QSpinBox, QDoubleSpinBox and QDateEdit and then extract the common code into functions.

Using QSpinBox as an example:

class RevertibleSpinBox : public QSpinBox
{
public:
    RevertibleSpinBox(QWidget* parent) : QSpinBox(parent)
    {
        RevertibleSpinBoxHelpers::installRevertAction(lineEdit(), this);
    }

public slots:
    void revert()
    {
        setValue(0);
    }

// etc.
};

namespace RevertibleSpinBoxHelpers
{
    void installRevertAction(QLineEdit* target, QObject* handler)
    {
        QAction* revertAction = new QAction(handler);
        target->addAction(revertAction, QLineEdit::TrailingAction);
        QObject::connect(revertAction, SIGNAL(triggered()), handler, SLOT(revert()));
    }
}

Disclaimer: If your needs are more complicated than you alluded to in your question, then this might not be the best approach.

Grunion

Thank you. This was one approach I used to overcome the signal/slots limitations of a template class. The templated base had a virtual connectAction() and the final class could implement it to connect its revert slot. – Cystine

@Kamajii: Is there a reason not to use this solution? – Victualer

@Silicomancer: too many classes created, at least 5 for now. I had the impression the OP did not want that, neither would I. – Osteopathy

Yes, that's nasty. But still I would prefer that solution. The decorator pattern seems the only alternative. But it is defined to be used on an interfaced. QAbstractSpinBox is abstract but it is NOT an interface. This is why the ghost-widget discussion arises. Personally I would prefer the 5 (clean) classes. If the number of classes is the problem it is actually only lazyness ;-) – Victualer

Answer 4

IMO you are complicating your life unnecessarily. Even the decorator pattern is overkill, much less all that needless inheritance and design woes. All you need is a nifty auxiliary function:

void revert(QAbstractSpinBox * box) {
    QSpinBox * spin = qobject_cast<QSpinBox *>(box);
    if (spin) { spin->setValue(spin->minimum()); return; }
    QDateTimeEdit * dt = qobject_cast<QDateTimeEdit *>(box);
    if (dt) { dt->setDateTime(dt->minimumDateTime()); return; }
    // and so on...
    qDebug() << "you should not be seeing this";
    return;
}

Wrap that as a static method of a Reverter object if you want to have something to instantiate alongside the spinbox.

You could make that a slot if you put it in a QObject derived class, but that's really not neccesary, since in Qt you can connect to free functions and methods of non-QObject derived classes. Or you can simply call it from slots if you so choose:

public slots:
    void revertSpinBoxA() { revert(spinBoxA); }
    void revertSpinBoxB() { revert(spinBoxB); }

This is similar to the decorator pattern, but it does come with a few advantages:

• it only provides the extra functionality for all objects of an inheritance hierarchy, which means you can save yourself the effort of having to reimplement existing functionality, you can directly use that of the object passed
• you only need one Reverter to revert any number of spinboxes, with the decorator you will need n number of decorators for n number of spinboxes, it will use up more memory, which might not be fatal but is needless overhead, since the type casting inside the revert method makes the actual decorator totally redundant. The difference is with the decorator, you will have the original object + a decorator for each one which you use as a controller for the object, while in this solution you still use the actual object, and only pass it through the Reverter for the extra functionality. Less work, less code, less complexity, less memory used.

Roee