Range-based for loop for a QList

tomy

Hello all,

Please take a look at my project.

ListAdd.h:

#ifndef LISTADD_H
#define LISTADD_H

#include <QDialog>
#include <QAbstractButton>

class QPushButton;
class QListWidget;
class QComboBox;
class QDialogButtonBox;

class ListAdd : public QDialog
{
    Q_OBJECT

public:
    ListAdd(QWidget *parent = nullptr);

private slots:
    void addData();
    void save();

private:
    void load();
    void btnClicked(QAbstractButton*);

    QComboBox* combo = nullptr;
    QPushButton* add = nullptr;
    QListWidget* list = nullptr;
    QDialogButtonBox* dBtBox = nullptr;
};
#endif // LISTADD_H

ListAdd.cpp:

#include "listadd.h"
#include <QListWidget>
#include <QComboBox>
#include <QPushButton>
#include <QDialogButtonBox>
#include <QHBoxLayout>
#include <QVBoxLayout>
#include <QSettings>
#include <QMessageBox>
#include <QVariant>

ListAdd::ListAdd(QWidget *parent)
    : QDialog(parent)
{
    combo = new QComboBox;
    combo->setEditable(true);
    add = new QPushButton(tr("&Add"));
    list = new QListWidget;
    list->setSelectionMode(QAbstractItemView::ExtendedSelection);

    dBtBox = new QDialogButtonBox(QDialogButtonBox::Ok |
                                  QDialogButtonBox::Cancel);
    dBtBox->setEnabled(true);
    dBtBox->addButton("Clear", QDialogButtonBox::ButtonRole::ActionRole);
    dBtBox->addButton("Remove", QDialogButtonBox::ButtonRole::ActionRole);

    QHBoxLayout* topLayout = new QHBoxLayout;
    topLayout->addWidget(combo);
    topLayout->addWidget(add);

    QVBoxLayout* mainLayout = new QVBoxLayout;
    mainLayout->addLayout(topLayout);
    mainLayout->addWidget(list);
    mainLayout->addWidget(dBtBox);

    setLayout(mainLayout);

    connect(add, &QPushButton::clicked, this, &ListAdd::addData);
    connect(dBtBox, &QDialogButtonBox::clicked, this, &ListAdd::btnClicked);
    load();
}

//*******************************************

void ListAdd::addData()
{
    if(combo->findText(combo->currentText()) < 0)
        combo->addItem(combo->currentText());

    list->addItem(combo->currentText());
}

//*************************************************************

void ListAdd::save()
{
    QSettings settings("Me", "CS Company");
    settings.clear();

    settings.setValue("text",combo->currentText());

    settings.beginWriteArray("combo");
    for (int i=0; i<combo->count();i++)
    {
       settings.setArrayIndex(i);
       settings.setValue("item", combo->itemText(i));
    }
    settings.endArray();

    settings.beginWriteArray("list");
    for (int i=0; i<list->count();i++)
    {
       settings.setArrayIndex(i);
       settings.setValue("item", list->item(i)->text());
    }
    settings.endArray();
}

//********************************************************

void ListAdd::load()
{
 QSettings settings("Me", "CS Company");

 combo->setCurrentText(settings.value("text", "").toString());

 int comboSize = settings.beginReadArray("combo");
 for (int i=0; i<comboSize;i++)
 {
    settings.setArrayIndex(i);
    combo->addItem(settings.value("item", "").toString());
 }
 settings.endArray();

 int listSize = settings.beginReadArray("list");
 for (int i=0; i<listSize;i++)
 {
    settings.setArrayIndex(i);
    list->addItem(settings.value("item", "").toString());
 }
 settings.endArray();
}

//********************************************************************

void ListAdd::btnClicked(QAbstractButton* button)
{
    if(button->text().contains("OK"))
    {
        save();
        QMessageBox::information(this, "Saved", "Your data is saved! Please re-open the application");
    }

    else if(button->text().contains("Cancel"))
    {
        QMessageBox::information(this, "Quit", "Your data is being lost");
        accept();
    }

    else if(button->text().contains("Clear"))
    {
        list->clear();
        combo->clear();
    }

    else if(button->text().contains("Remove")) {
        QList<QListWidgetItem*> items = list->selectedItems();

        for(auto& item : items)
        {
            list->removeItemWidget(item);
            delete item;
        }
    }
}

The question is about the else-if condition above, which is, why without delete item;, won't the item be removed from the list, please?

Christian Ehrlicher

@tomy said in Range-based for loop for a QList:

The question is about the else-if condition above, which is, why without delete item;, won't the item be removed from the list, please?

It's perfectly written in the documentation.

tomy

@Christian-Ehrlicher

What does "entirely" there mean?

And the mysterious thing is that the first time I used:

for(QListWidgetItem* item: items)

And it used to work (remove the row from the list) without "delete", but not now.

And one rather side-question, please. I think except for some rare situations like above that we manually need to delete a pointer, with the parent-child relationship, we needn't have to use a destructor in the projects. Right?

jsulm

@tomy Did you read this from the link:

To remove an item (row) from the list entirely, either delete the item or use takeItem().

?

Bonnie

@tomy I think you must have used a different function.
removeItemWidget does not remove a item.
It removes the widget set by setItemWidget.
If there's no widget set, it does nothing.
But even you remove a item with takeItem, you still need to delete it manually.

Asperamanca

@tomy said in Range-based for loop for a QList:

    for(auto& item : items)
    {
        list->removeItemWidget(item);
        delete item;
    }

Not sure it's safe to delete an item while looping over the list
Safer to do:

        for(auto& item : items)
        {
            list->removeItemWidget(item);
        }
        qDeletAll(items);

tomy

I hope I get answers for the last two questions I asked, "entirely" and "destructor". Should I make separate threads for them?

@Bonnie
Well, there's no widget set by setItemWidget; I used selectionItems. So you mean that function neither removes any item in the list nor is it needed in the code?

@Asperamanca

It may make sense not to delete the cells of the list while you're still connected to it, but except for that is there any rationale?
So if I use this for that last condition, it's the easiest and best method!

else if(button->text().contains("Remove")) {
        qDeleteAll(list->selectedItems());

Bonnie

@tomy Yes, if you open qlistwidget.h, you can see that:

inline void QListWidget::removeItemWidget(QListWidgetItem *aItem)
{ setItemWidget(aItem, nullptr); }

tomy

@Bonnie

I took a look twice on the task that removeItemWidget does, but couldn't understand this, since the setItemWidget looks complicated.
Will you explain what that remove function does in my code, please. This way it becomes clearer I suppose.

Christian Ehrlicher

@Asperamanca said in Range-based for loop for a QList:

Safer to do:

In this case it's not needed - it just adds another loop for nothing.

Christian Ehrlicher

@tomy said in Range-based for loop for a QList:

Will you explain what that remove function does in my code, please.

It removes the widget from the view, nothing more (and exactly what the function name states...)

tomy

@Christian-Ehrlicher

By "from the view" you mean "disappearing" right? But it does nothing to be seen.

Bonnie

@tomy As I said, if you haven't called setItemWidget to set a widget, then it does nothing at all.
[ADDED]
And setItemWidget is not that complicated.
You can use it to set a widget to a item, maybe a button, a combox, or any kind of widget.
When the item is visible in the list view, the widget will be shown on the item rect.
So by removeItemWidget, you can remove that widget.

Asperamanca

This post is deleted!

Asperamanca

This post is deleted!

tomy

@Bonnie

I've used "addItem" for both the comboBox and listWidget to add items thereby they are visible, but removeItem is for this usage apparently and deleting the selected items will do the job.
Thanks.