2

I think I have some major problem with a concept that seems pretty basic to me.

I created a custom widget, which is actually just a small collection of widgets, which, as such, would appear several times.

class CustomWidget : public QWidget {
    Q_OBJECT
public:
    explicit CustomWidget(QWidget parent=nullptr) : QWidget(parent) {
        spinboxA = new QSpinBox;
        spinboxB = new QSpinBox;
        QHBoxLayout* layout = new QHBoxLayout(this);
        layout.addWidget(spinboxA);
        layout.addWidget(spinboxB);
        this->setLayout(layout);
    }
private:
    QSpinBox* spinboxA;
    QSpinBox* spinboxB;
};

This custom widget is then used inside a gui. I want this gui to react to changes of the value of the spinboxes, of course. In my understanding I can either

1) Provide getter for the QSpinBoxes and connect their signals outside the class. 2) "Re-route" their signals like in the example below

@1) used via connect(customwidget->getSpinboxA(),SIGNAL(valueChanged(int)),this,SLOT(doSomething(int)));, I guess?

@2)

class CustomWidget : public QWidget {
    Q_OBJECT
public:
    explicit CustomWidget(QWidget parent=nullptr) : QWidget(parent) {
        spinboxA = new QSpinBox;
        spinboxB = new QSpinBox;
        QHBoxLayout* layout = new QHBoxLayout;
        layout.addWidget(spinboxA);
        layout.addWidget(spinboxB);
        this->setLayout(layout);
        connect(spinboxA,SIGNAL(valueChanged(int)),//...
            this,SLOT(onSpinboxAValueChanged(int)));
    }
private:
    QSpinBox* spinboxA;
    QSpinBox* spinboxB;
private slots:
    void onSpinboxAValueChanged(int x) {emit spinboxAValueChanged(x);}
    //...
signals:
    void spinboxAValueChanged(int x)
};

In the gui class one would connect(customwidget,SIGNAL(spinboxAValueChanged(int),this,SLOT(doSomething(int)));

Especially version 2) seems very cluttered and... I'm asking myself - how do I connect to the signals of widgets inside my custom widget?

scopchanov
  • 7,966
  • 10
  • 40
  • 68
LCsa
  • 607
  • 9
  • 30

1 Answers1

4

A CustomWidget should be modular, that is, it should be like a black box, where inputs should be established and outputs obtained, so for me the second solution is very close to it, but I see something that can be improved: it is not necessary to create a slot only to emit a signal, signals can be connected to other signals, I also recommend using the new connection syntax.

#include <QApplication>
#include <QHBoxLayout>
#include <QSpinBox>
#include <QWidget>

#include <QDebug>

class CustomWidget : public QWidget {
    Q_OBJECT
public:
    explicit CustomWidget(QWidget *parent =nullptr):
        QWidget(parent),
        spinboxA(new QSpinBox),
        spinboxB(new QSpinBox)
    {
        QHBoxLayout* layout = new QHBoxLayout(this);
        layout->addWidget(spinboxA);
        layout->addWidget(spinboxB);
        connect(spinboxA, QOverload<int>::of(&QSpinBox::valueChanged), this, &CustomWidget::spinboxAValueChanged);
        connect(spinboxB, QOverload<int>::of(&QSpinBox::valueChanged), this, &CustomWidget::spinboxBValueChanged);
        // old syntax:
        // connect(spinboxA, SIGNAL(valueChanged(int)), this, SIGNAL(spinboxAValueChanged(int)));
        // connect(spinboxB, SIGNAL(valueChanged(int)), this, SIGNAL(spinboxBValueChanged(int)));
    }
private:
    QSpinBox *spinboxA;
    QSpinBox *spinboxB;
signals:
    void spinboxAValueChanged(int x);
    void spinboxBValueChanged(int x);
};

int main(int argc, char *argv[])
{
    QApplication a(argc, argv);
    CustomWidget w;

    QObject::connect(&w, &CustomWidget::spinboxAValueChanged, [](int i){
       qDebug()<< "spinboxAValueChanged: "<< i;
    });
    QObject::connect(&w, &CustomWidget::spinboxBValueChanged, [](int i){
       qDebug()<< "spinboxBValueChanged: "<< i;
    });

    w.show();

    return a.exec();
}

#include "main.moc"
eyllanesc
  • 235,170
  • 19
  • 170
  • 241
  • Thank you! How would you do it in the old syntax? Just for the completeness of syntax knowledge? :) – LCsa Sep 09 '18 at 20:03
  • Ah...that's actually quite simple, thank you. Why is the new syntax recommended? – LCsa Sep 09 '18 at 20:22
  • Notification-only APIs are hard to use: you almost always end up with some code that caches the value. Thus, in addition to the notifier, there should also be a getter. I'd go even further and declare the values to be properties (using `Q_PROPERTY`) to clearly indicate that there is a read-only value with available notification and getter. This indicates intent to the human maintainer, as well as makes it easy to use the class from scripting languages, including Qt's own javascript engine. – Kuba hasn't forgotten Monica Sep 12 '18 at 13:04