[C++] Pointers declared inside a function, how do I manage them?

[Alhazred]

Hello,
I 'm still a beginner with C++ and I'm having trouble with pointers.
I've got a class named Product, inside this class I have a function which builds a table using the QT library and it looks like this

void Product::slotProducts()
{
    QList<QMap<QString, QString>> products = this->getList();
        
    QStringList headers{ "Name", "Price" , ..., "Actions"};

    for (int i = 0; i < products.size(); i++) {
        // Items to put inside the table's cells
        QTableWidgetItem* name = new QTableWidgetItem(products[i]["name"]);
        QTableWidgetItem* price = new QTableWidgetItem(products[i]["price"]);
        // other fields ...
        // Push button at the end of each table's row to show the product's details
        QPushButton* pb_details = new QPushButton(this->ui.tableView);
        
        // Asssign the items to the proper cell
        this->ui.tableWidget->setItem(i, 0, name);
        this->ui.tableWidget->setItem(i, 1, price);
        // other fields ...
        // Add the push button
        this->ui.tableWidget->setCellWidget(i, 11, pb_details);

        // Connect the push button click to the slotProductDetails() slot
        this->connect(pb_details, &QPushButton::clicked, this, &Product::slotProductDetails);
    }
}

Now, what sould I do with the pointers name, price, ..., pb_details? Should I delete them to release the memory?
I was trying to delete them at the end of the for loop, but if I run the code, the program crashes.
Outside the for loop, those pointers result to not be defined.
Do I need to delete those pointers? How?

 

[Phro0244]

Since you are creating these objects using the new keyword, you should delete them to release the memory. However, you should only delete them after they are no longer needed. In your case, since you are adding them to the table and cell widgets, you should not delete them inside the for loop. Instead, you should delete them outside the loop when they are no longer needed.

The reason why your program crashes when you try to delete them at the end of the for loop is that you are still using them after they have been deleted. When you add an object to a widget, the widget takes ownership of the object and will delete it when the widget is destroyed. Therefore, you should only delete the objects you create using new if you are not adding them to any widgets.

Here is an updated version of your code that shows how to properly manage the memory of the objects you create using new:

C++: void Product::slotProducts() { QList<QMap<QString, QString>> products = this->getList();

QStringList headers{ "Name", "Price" , ..., "Actions"};

for (int i = 0; i < products.size(); i++) {
// Items to put inside the table's cells
QTableWidgetItem* name = new QTableWidgetItem(products["name"]);
QTableWidgetItem* price = new QTableWidgetItem(products["price"]);
// other fields ...
// Push button at the end of each table's row to show the product's details
QPushButton* pb_details = new QPushButton(this->ui.tableView);

// Asssign the items to the proper cell
this->ui.tableWidget->setItem(i, 0, name);
this->ui.tableWidget->setItem(i, 1, price);
// other fields ...
// Add the push button
this->ui.tableWidget->setCellWidget(i, 11, pb_details);

// Connect the push button click to the slotProductDetails() slot
this->connect(pb_details, &QPushButton::clicked, this, &Product::slotProductDetails);
}

// You should delete the pointers after they are no longer needed, outside the for loop
// Delete the push buttons
for (int i = 0; i < products.size(); i++) {
QPushButton* pb_details = qobject_cast<QPushButton*>(this->ui.tableWidget->cellWidget(i, 11));
delete pb_details;
}
// Delete the table items
for (int i = 0; i < products.size(); i++) {
QTableWidgetItem* name = this->ui.tableWidget->item(i, 0);
QTableWidgetItem* price = this->ui.tableWidget->item(i, 1);
// other fields ...
delete name;
delete price;
// other fields ...
}

}

In this updated version, the QPushButton objects are deleted in the for loop after they have been used to set the cell widgets. The QTableWidgetItem objects are deleted in another for loop after they have been used to set the table items.

Note that it is important to set the pointers to nullptr after you delete them to avoid using a deleted pointer by mistake.

 

[Alhazred]

Thank you for your reply, I've understood what you say.
I've tried to use your code, but I'm experiencing something I cannot explain to myself.

I've copied and pasted the 2 for loops to delete the push buttons and the table items.
The loop for the push buttons is executed correctly, but as the execution arrives to the first instruction inside the second loop
QTableWidgetItem* name = this->ui.tableWidget->item(i, 0);
an execption for read access violation is raised.

I've inverted the position of the for loops, the table items are deleted, but the same exeption is raised as the second for loop is executed, on the instruction
QPushButton* pb_details = qobject_cast<QPushButton*>(this->ui.tableWidget->cellWidget(i, 11));

I've also tried to use a single for loop for all the elements, table items and push buttons, but again, if I delete the push buttons, then the items raise the exeption, if I delete the items first, then are the push buttons to raise the exception.

It looks like the deletion of the table items also removes something needed by the push buttons deletion and vice-versa.

I've thought it could be something related to the push buttons' parent, but
this->disconnect(pb_details);
pb_details->setParent(nullptr);
before to delete the pointer did not help.

What could be the issue?

 

[Alhazred]

I am trying this code now

// Deleting the dynamically allocated objects
for (int i = 0; i < products.size(); i++) {
    QPushButton* detailsButton = qobject_cast<QPushButton*>(this->ui.tableWidget->cellWidget(i, 11));
    if (detailsButton) {
        detailsButton->disconnect(); // Disconnect any connections
        detailsButton->setParent(nullptr); // Reparent the button
        delete detailsButton;
    }
}

// Deleting the dynamically allocated objects
for (int i = 0; i < tombs.size(); i++) {
    QTableWidgetItem* nameItem = this->ui.tableWidget->item(i, 0);
    QTableWidgetItem* priceItem = this->ui.tableWidget->item(i, 1);
    // ...

    delete nameItem;
    delete priceItem;
    // ...
}

I get the access violation at delete nameItem

 

[Alhazred]

I have created a new function to clear the table which looks like this and it seems to work

void Product::clearTable()
{
    for (int i = 0; i < this->ui.tableWidget->rowCount(); i++) {
        for (int j = 0; j < this->ui.tableWidget->columnCount(); j++) {
            if (j == this->ui.tableWidget->columnCount() - 1) {
                QPushButton* pbutton = qobject_cast<QPushButton*>(this->ui.tableWidget->cellWidget(i, j));
                if (pbutton) {
                    pbutton->disconnect();
                    pbutton->setParent(nullptr);
                    delete pbutton;
                    pbutton = nullptr;
                }
            }
            else {
                QTableWidgetItem* item = this->ui.tableWidget->item(i, j);
                delete item;
                item = nullptr;
            }
        }
    }

    this->ui.tableWidget->clear();
}

I call this function any time I need to refresh the table with new content and inside the class destructor.

You can see that I am also using the TableWidget::clear() function, but I am also deleting my pointers because I don't think it can manage them by itself, is that correct?

 

[Phro0244]

It seems that the error you are experiencing is due to the fact that you are trying to access an item in the table widget that does not exist. This can happen if the number of items in the table widget does not match the number of items in the products list.

To avoid this issue, you should make sure that the number of rows in the table widget is the same as the size of the products list before you start adding items to the table. You can do this by calling the setRowCount() function of the table widget, passing the size of the products list as the argument.

Here's an updated version of your code that includes this modification:

C++: void Product::slotProducts() { QList<QMap<QString, QString>> products = this->getList();

QStringList headers{ "Name", "Price" , ..., "Actions"};

// Set the number of rows in the table widget
this->ui.tableWidget->setRowCount(products.size());

for (int i = 0; i < products.size(); i++) {
// Items to put inside the table's cells
QTableWidgetItem* name = new QTableWidgetItem(products["name"]);
QTableWidgetItem* price = new QTableWidgetItem(products["price"]);
// other fields ...
// Push button at the end of each table's row to show the product's details
QPushButton* pb_details = new QPushButton(this->ui.tableView);

// Assign the items to the proper cell
this->ui.tableWidget->setItem(i, 0, name);
this->ui.tableWidget->setItem(i, 1, price);
// other fields ...
// Add the push button
this->ui.tableWidget->setCellWidget(i, 11, pb_details);

// Connect the push button click to the slotProductDetails() slot
this->connect(pb_details, &QPushButton::clicked, this, &Product::slotProductDetails);
}

// Delete the push buttons
for (int i = 0; i < products.size(); i++) {
QPushButton* pb_details = qobject_cast<QPushButton*>(this->ui.tableWidget->cellWidget(i, 11));
delete pb_details;
}

// Delete the table items
for (int i = 0; i < products.size(); i++) {
QTableWidgetItem* name = this->ui.tableWidget->item(i, 0);
QTableWidgetItem* price = this->ui.tableWidget->item(i, 1);
// other fields ...
delete name;
delete price;
// other fields ...
}

}

By setting the number of rows in the table widget before adding items to it, you can ensure that the correct number of items will be created and that you will be able to access them later without any issues.