Qt. signal linked to slot of latest object

Viewed 59

I am creating multiple object of type QWidget_WindowContact at runtime. When I click the Increment or decrement buttons the value in the last generated object gets updated and not the value in the same object.

I am struggling to properly link the signals and slots, so that when multiple objects of the same type are generated at runtime the signal emitted by the button coressponds to the signal in the right object.

QWidget_WindowContact.h:

#ifndef QWIDGET_WINDOWCONTACT_H
#define QWIDGET_WINDOWCONTACT_H

#include "qwidget.h"

class QWidget_WindowContact : public QWidget
{

Q_OBJECT

public:
    explicit QWidget_WindowContact(QWidget* parent = nullptr);

    private slots:
        void on_btnInc_clicked();
        void on_btnDec_clicked();
        void on_btnDel_clicked();

private:
    QPoint offset;

protected:

};

#endif // QWIDGETTEMP_H

QWidget_WindowContact.h:

#include <QtWidgets>

#include "QWidget_WindowContact.h"

QSpinBox* counter;

QWidget_WindowContact::QWidget_WindowContact(QWidget* parent) : QWidget(parent)
{
    this->setObjectName("");
    setMinimumSize(100, 100);

    this->setStyleSheet("border: 1px solid gray");

    QVBoxLayout* layout = new QVBoxLayout(this);

    QLabel* name = new QLabel("WINDOWCONTACT", this);

    QPushButton* btnInc = new QPushButton("Increment", this);
    btnInc->setObjectName("btnInc");
    QPushButton* btnDec = new QPushButton("Decrement", this);
    btnDec->setObjectName("btnDec");
    QPushButton* btnDel = new QPushButton("Delete Widget", this);
    btnDel->setObjectName("btnDel");

    counter = new QSpinBox(this);
    counter->setObjectName("counter");

    connect(this->findChild<QPushButton*>("btnInc"), SIGNAL(clicked()), this, SLOT(on_btnInc_clicked()));
    connect(this->findChild<QPushButton*>("btnDec"), SIGNAL(clicked()), this, SLOT(on_btnDec_clicked()));
    connect(this->findChild<QPushButton*>("btnDel"), SIGNAL(clicked()), this, SLOT(on_btnDel_clicked()));

    layout->addWidget(name);
    layout->addWidget(counter);
    layout->addWidget(btnInc);
    layout->addWidget(btnDec);
    layout->addWidget(btnDel);

}

void QWidget_WindowContact::on_btnInc_clicked()
{
    counter->setValue(counter->value() + 1);
}

void QWidget_WindowContact::on_btnDec_clicked()
{
    counter->setValue(counter->value() - 1);
}

void QWidget_WindowContact::on_btnDel_clicked()
{
    close();
}
2 Answers

You have

QSpinBox* counter;

as a global variable, which changes its value every time new QWidget_WindowContact is created. This piece of code:

void QWidget_WindowContact::on_btnInc_clicked()
{
    counter->setValue(counter->value() + 1);
}

is using this global variable, which is always the pointer to the most recently created QSpinBox. Remove the global variable and either keep this QSpinBox* ptr as a private member of your class, or use findChild QT feature:

void QWidget_WindowContact::on_btnInc_clicked()
{
    QSpinBox* counter = this->findChild<QSpinBox*>("counter");
    counter->setValue(counter->value() + 1);
}

As a side note, findChild is not actually needed in your CTor, because you still have the pointers to corresponding objects in scope.

Generally speaking, setting the widget names is not necessary unless you need it for debugging tools or introspection. Naming can be easily added at construction time using a template class, as well as insertion into the layout. That cuts down on verbosity. Note that WQ is a library class that you'd write once, and use widely, so there's not much concern with how "convoluted" it is - although of course it should use an actual tag-dispatch header library to

#include <QtWidgets>

static inline constexpr auto name_ = [] {};
static inline constexpr auto layout_ = [] {};
static inline constexpr auto text_ = [] {};

template<class Base>
class WQ : public Base
{
public:
    WQ() = default;

    template<class... Args>
    WQ(decltype(layout_), QLayout *layout, Args &&...args) : WQ(std::forward<Args>(args)...)
    {
        layout->addWidget(this);
    }

    template<class... Args>
    WQ(decltype(name_), QStringView name, Args &&...args) : WQ(std::forward<Args>(args)...)
    {
        this->setObjectName(name.toString());
    }

    template<class... Args>
    WQ(decltype(text_), QStringView text, Args &&...args) : WQ(std::forward<Args>(args)...)
    {
        this->setText(text.toString());
    }
};

The widget can be very compact, and built up declaratively - don't feel inspired by the code that comes as Qt examples, it's unnecessarily verbose. We can set up most "short" widget properties in the body of the class:

class QWidget_WindowContact : public QWidget
{
    Q_OBJECT
    using Self = QWidget_WindowContact;

public:
    explicit QWidget_WindowContact(QWidget *parent = nullptr);

private:
    void inc_clicked();
    void dec_clicked();

    QPoint offset;
    QVBoxLayout layout{this};
    WQ<QLabel> name{::layout_, &layout, ::text_, tr("WINDOWCONTACT")};
    WQ<QPushButton> btnInc{::layout_, &layout, ::text_, tr("Increment")};
    WQ<QPushButton> btnDec{::layout_, &layout, ::text_, tr("Decrement")};
    WQ<QPushButton> btnDel{::layout_, &layout, ::text_, tr("Delete Widget")};
    WQ<QSpinBox> counter{::layout_, &layout, ::name_, L"counter"};
};

Then, the constructor is simple as well:

QWidget_WindowContact::QWidget_WindowContact(QWidget *parent) : QWidget(parent)
{
    setMinimumSize(100, 100);
    setStyleSheet("border: 1px solid gray");

    connect(&btnInc, &QPushButton::clicked, this, &Self::inc_clicked);
    connect(&btnDec, &QPushButton::clicked, this, &Self::dec_clicked);
    connect(&btnDel, &QPushButton::clicked, this, &Self::close);
}

The slots remain as they were, and there's no need to actually make them slots unless you need that for introspection. Don't wrap methods in slots that "do nothing" but just forward the call - it's unnecessary. Also note the use of modern (Qt 5) style of signal-slot connections. Also note that the current Qt version is Qt 6.1 - so the modern signal-slot syntax is hardly "modern".

void QWidget_WindowContact::inc_clicked()
{
    counter.setValue(counter.value() + 1);
}

void QWidget_WindowContact::dec_clicked()
{
    counter.setValue(counter.value() - 1);
}

The example sets up a 3x3 grid of the widgets within an outer window:

int main(int argc, char *argv[])
{
    QApplication a(argc, argv);
    QWidget w;
    QGridLayout layout{&w};
    constexpr int rows = 3, cols = 3;
    QWidget_WindowContact wc[rows * cols];
    for (int i = 0; i < rows; ++i)
        for (int j = 0; j < cols; ++j)
            layout.addWidget(&wc[i * cols + j], i, j);
    w.show();
    return a.exec();
}

#include "main.moc"

This concludes the compileable example. It looks like this:

Screenshot of the example

Related