Correct use of Signals & Lambdas

[J.Hilk]

Hello everyone,

I have finally found my why to the "new" Signal&Slot Syntax that QT5 brought with it. Now that I know how to use it, I find myself using it more and more and a question came up.

First, let me post a quick code-section form my latest project. With it I create a series of popup-buttons in a circle around the clicked widget.

for(int j = 0; j < 3; j++){

            cLabel *cLbl = new cLabel(ui->Produkt);
            cLbl->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Expanding);
            cLbl->resize(ui->stackWidgetMain->width()/12, ui->stackWidgetMain->width()/12);
            cLbl->setStyleSheet("background-color:transparent;");

            connect(this,&HauptDesign::eventResize,[=]{cLbl->hide();});
            cLbl->hide();

            QPushButton *btn = ui->Produkt->findChild<QPushButton *> ("btnProdukt_"+QString::number(i+1));
            if(btn){
                connect(btn, &QPushButton::pressed, [=]{emit hideProductSelection();});
                switch(j){
                case 0:
                    connect(btn, &QPushButton::clicked, [=]{ cLbl->show();
                        cLbl->resize(ui->stackWidgetMain->width()/12, ui->stackWidgetMain->width()/12);
                        cLbl->setPixmap(QPixmap(":/images/Menu_Connect.png").scaled(QSize(cLbl->size()),Qt::KeepAspectRatio));
                        cLbl->move(widget->x()+(widget->width()-cLbl->width())/2, widget->y()-(cLbl->height()*3/6));
                    });break;
                case 1:
                    connect(btn, &QPushButton::clicked,[=]{ cLbl->show();
                        int xMove = (qSin(120)*widget->height())/2; int yMove = (qCos(120)*widget->height())/4;
                        cLbl->resize(ui->stackWidgetMain->width()/12, ui->stackWidgetMain->width()/12);
                        cLbl->setPixmap(QPixmap(":/images/Menu_List.png").scaled(QSize(cLbl->size()),Qt::KeepAspectRatio));
                        cLbl->move(widget->x()+(widget->width()/2)-xMove - cLbl->width(), widget->y()+widget->height()/2+yMove);
                    });break;
                case 2:
                    connect(btn, &QPushButton::clicked,[=]{ cLbl->show();
                        int xMove = (qSin(120)*widget->height())/2; int yMove = (qCos(120)*widget->height())/4;
                        cLbl->resize(ui->stackWidgetMain->width()/12, ui->stackWidgetMain->width()/12);
                        cLbl->setPixmap(QPixmap(":/images/Menu_SpeichernLaden.png").scaled(QSize(cLbl->size()),Qt::KeepAspectRatio));
                        cLbl->move(widget->x()+(widget->width()/2)+xMove /*+ cLbl->width()/4*/, widget->y()+widget->height()/2+yMove);
                    });break;
                }
                connect(this,&HauptDesign::hideProductSelection, cLbl, &cLabel::hide);
            }
        }

and it actually works as intendet.

My question is, do I create some kind of memory leak with this?

As you can see, I connect Signals zu Lambda-functions that refer to pointer of objects that are only created "temporarly" in this loop.
Usually you can't access those Objects without a permanent pointer - created in the header file - to whom you link the object to. Or with a findchild operation or a @static_cast<QObject*>sender()@ later on.

It works and it is super easy, but it feels kinda "unclean coding"?

So can I use the Signal/Lambda-Connecten like this or should I go an other way?

Greetings

[jsulm]

@J.Hilk As you set a parent there should not be any memory leak.
Yes, the pointer variable is local, but the objects are not created "temporarily" as you allocate them on the heap. The pointer variables go out of scope later, but that isn't an issue.

[VRonin]

the magic is all inside [=] when you create the lambda that pointer is passed by value and stored in the lambda so it can be used in the future even though the pointer itself is a temporary value. It's completely clean code.

That said, do I like your code? no. Using a counter j just to use it in a switch makes the whole loop useless in practice. You also create a new label every time (cLabel *cLbl = new cLabel(ui->Produkt);) even if then you just hide it.

So in the end;

• are you creating memory leaks? no
• are you using lambdas correctly? yes
• are you likely to flood the memory? yes as you continue to create new temporary objects even if you have one already and it's just hidden
• is your code design clear? No, but that's just my personal opinion

[J.Hilk]

Thanks for the quick reply guys!

This should answer my questions.

Also to be more specific, this is part of a setup/initialization-code so this objects and connections are created once during startup, so I should not flood the memory. The main loop of widgets is about 10 in size.

But I see your point about the switch statement, I could wrap everything up in one connect-statement, but than I would have to create the 3 different Icons berforehand, to refer them correctly in the lambda.

I may do that, was never a fan of switches in loops myself either.

Anyway thanks alot.

Greetings!