QT signal slot not working

Question

Why this code does not call the desired slot?

I have trying to make it work for the last 3 hours!

void TForm::on_clicked( bool checked )
{
    QMessageBox *messageBox = new QMessageBox(this);
    QPushButton *buttonAccept0 = new QPushButton("OK", messageBox);
    QPushButton *buttonReject = new QPushButton("Cancel", messageBox);
    messageBox->addButton(buttonAccept0, QMessageBox::ButtonRole::AcceptRole);
    messageBox->addButton(buttonReject, QMessageBox::ButtonRole::NoRole);
    messageBox->setWindowTitle("Confirmation");
    messageBox->setText("Remove?");
    messageBox->setModal(true);
    messageBox->show(); 
    QObject::connect(buttonAccept0, SIGNAL(accepted()), this, SLOT(a1()));
}
void TForm::a1()
{
    std::string ff = "";
}

Answer 1

There are three things worth noting about your question:

---

Firstly, your error is as a result of you using the incorrect signal (as other members have already mentioned).

To correct your error, make the following change to your code:

QObject::connect(buttonAccept0, SIGNAL(clicked()), this, SLOT(at()));

---

Secondly, unlike @M74 mentioned in his/her answer, it is perfectly allowed to declare the message box and the signals inside the on_clicked function. The reason @M74 is concerned is because of potential memory leaks but Qt has a mechanism for overcoming that in the form of parenting, which you did correctly when you defined the messagebox at the beginning of the function.

Unfortunately, @M74's concerns are not entirely unfounded since the messagebox will only be destroyed when the parent is destroyed.

If your instance of TForm remains alive for the entire time of the application, and you execute on_clicked many times, you will have many instances of the messageBox created and waiting in the background to be deleted, a kind of memory leak in itself.

A better way of avoiding this would be to modify your code to make the messageBox a stack variable, local to the function. The two QPushButton instances can still be created in heap memory since they will be automatically destroyed when the messagebox is destroyed, once again because of parenting.

void TForm::on_clicked(bool checked)
{
    QMessageBox messageBox;
    QPushButton *buttonAccept0 = new QPushButton("Ok", &messageBox);
    QPushButton *buttonReject = new QPushButton("Cancel", &messageBox);
    messageBox.addButton(buttonAccept0, QMessageBox::AcceptRole);
    messageBox.addButton(buttonReject, MessageBox::NoRole);
    messageBox.setWindowTitle("Confirmation");
    messageBox.setText("Remove?");
    messageBox.setModal(true);
    connect(buttonAccept0, SIGNAL(clicked()), this, SLOT(a1()));
    messageBox.show();
}

Note:

1. It is generally a good idea to set up all your connections before displaying your dialog boxes, in case a signal fires before you've made the connection. This is why I moved the show statement to last.

2. You can replace the two statements:

   messageBox.setModal(true);

   messageBox.show();

with

    messageBox.exec();

the result will be the same.

---

Lastly,

QMessageBox comes with a number of static functions for common purposes. The code that is shown in the question fits perfectly into one of the standard use-cases so everything can be rewritten using the following:

if (QMessageBox::question(nullptr, "Confirmation", "Remove?", 
    QMessageBox::Ok | QMessageBox::Cancel, 
    QMessageBox::Cancel) == QMessageBox::Ok)
{
    a1();
}