0

In a GUI application, I have quite a few pushbuttons that are being used. These pushbuttons are labelled as pbxx where xx is the row and column number of the pushbutton in a grid layout. When a pushbutton is pressed, it needs to be highlighted. Today I read about lambda functions Brian Poteat & Kuba Ober and thought I would try to implement this in my code.

In my GuiDisplay class (a class that inherits QMainWindow), I have a function called:

make_connections(); 

which would make all my pushbutton connections (all signals are connected to a single slot on_pushbutton_clicked(). I added the code here:

The GuiDisplay Class

class GuiDisplay : public QMainWindow
{
    Q_OBJECT

public:
    explicit GuiDisplay(QWidget *parent = 0);
    ~GuiDisplay();

    ... Some more public functions ...

    /**
     * @brief Connects all custom signals and slots.
     */
    void make_connections();

    /**
     * @brief Will highlight the provided pushbutton with provided rgb colour
     */
    void highlight_pushbutton(QPushButton &pb, const int rgb[]);

private slots:
    void on_pushbutton_clicked(const QString& label);

private:
    Ui::GuiDisplay *ui;
};

The make_connections function of the GuiDisplay Class

void GuiDisplay::make_connections()
{
    // Loop through all the pushbuttons and connect clicked signal to clicked slot
    QString pb_label = "";
    for(int i = 0; i < 8; ++i)
    {
        for(int j = 0; j < 8; ++j)
        {
            // Find the pushbutton
            pb_label = "pb" + QString::number(i) + QString::number(j);
            QPushButton* pb_ptr = this->findChild<QPushButton*>(pb_label);
            //connect(pb_ptr, SIGNAL(clicked()), this, SLOT(on_pushbutton_clicked()));
            connect(pb_ptr, &QPushButton::clicked, [this]{on_pushbutton_clicked(pb_label);});
        }
    }
}

The problem comes up when I make the connection

connect(pb_ptr, &QPushButton::clicked, [this]{on_pushbutton_clicked(pb_label);});

the build gives me the following error

'pb_label' is not captured

so I thought ok, well then it wouldn't seem wrong to do the following instead:

connect(pb_ptr, &QPushButton::clicked, [this, &pb_label]{on_pushbutton_clicked(pb_label);});

The error at build time fell away, however my GUI application unexpectedly crashes whenever this code is executed. Not sure why. Any help here?

Community
  • 1
  • 1
effort
  • 107
  • 1
  • 10

2 Answers2

4

Your error is due to using a reference to a temporary. pb_label only exists for the duration of GuiDisplay::make_connections your lambda is stored by Qt and will be referenced each time the signal is fired. When that happens you'll reference a destroyed object.

So the correct way to make the connection is:

connect(pb_ptr, &QPushButton::clicked, [=]{ on_pushbutton_clicked(pb_label); });

As of Qt 5.2 the QMetaObject::Connection QObject::connect(const QObject *sender, PointerToMemberFunction signal, const QObject *context, Functor functor, Qt::ConnectionType type = Qt::AutoConnection) overload was added which will automatically disconnect at the end of an object's lifetime. In this case the connection would be:

connect(pb_str, &QPushButton::clicked, this, [=]{ on_pushbutton_clicked(pb_label); });

That said you shouldn't use a lambda here when you could simply do:

connect(pb_ptr, &QPushButton::clicked, this, &GuiDisplay::on_pushbutton_clicked);

Then modify your GuiDisplay::on_pushbutton_clicked to take no arguments. You can obtain the name of the object in GuiDisplay::on_pushbutton_clicked by calling: sender() which will:

Returns a pointer to the object that sent the signal

Then just use QObject::objectName to get pb_label:

This property holds the name of this object

So, you'll just need to replace pb_label with: sender()->objectName()

There are several reasons why the direct slot connection is preferable to a lambda:

  1. It removes the need to store a temporary lambda and a temporary QString for pb_label
  2. It removes the lambda indirection that requires an additional stack frame
  3. Debugging into lambdas is not always developer friendly, though some IDEs mitigate this problem
  4. Prior to Qt 5.2 there was no way to disconnect a lambda slot if objects it depended upon were destroyed:

There is no automatic disconnection when the 'receiver' is destroyed because it's a functor with no QObject.

Jonathan Mee
  • 37,899
  • 23
  • 129
  • 288
  • 1
    I think it is worth mentioning, that assuming Qt 5.2 or newer is used, if you need automatic disconnection when some object is destroyed, you can pass that object as the [`context object`](http://doc.qt.io/qt-5/qobject.html#connect-5). – thuga Mar 24 '16 at 14:19
  • @thuga Fair point, I actually even censored that from my quote of the Qt wiki. I never bothered to upgrade from Qt 5.1 and I was working under the non-sense mindset that no one is using Qt 5.2. I've updated the answer. – Jonathan Mee Mar 24 '16 at 14:51
1

Thank you Piotr Skotnicki and Simple, both of your suggestions work. I probably should have read a bit further and I would have seen [=] in the capture-list (See: Lambda)

So, my one line of code that causes the problem can be either:

connect(pb_ptr, &QPushButton::clicked, [this, pb_label]{on_pushbutton_clicked(pb_label);});

or

connect(pb_ptr, &QPushButton::clicked, [=]{on_pushbutton_clicked(pb_label);});
effort
  • 107
  • 1
  • 10
  • 2
    FWIW, when it's captured by reference, `[this, &pb_label]`, then the original object (that the captured reference points to) is destroyed on an exit from `make_connections`, that's why you encounter crashes – Piotr Skotnicki Mar 24 '16 at 11:10