Automatically deletion of Child Widgets in QVector is not happening

sandro4912

So I have this Widget Minefield which has a Vector with Cell widgets as member:

class Minefield : public QWidget
{
//...
private:
QVector<Cell *> mCells;
}

class Cell : public QWidget
{
    Q_OBJECT
public:
//....

    Cell(State state, QWidget *parent = nullptr);

The Cells get populated on startup:

Minefield::Minefield(int width,
                     int height,
                     int countOfMines,
                     QWidget *parent)
    :QWidget{ parent },
//....
      mCells{ createCells(width, height, countOfMines) },
//...
{
///....
}

QVector<Cell *> Minefield::createCells(
        int width, int height, int countOfMines)
{
    QVector<Cell *> cells;
    auto size = static_cast<int>(width * height);
    cells.reserve(size);

    for(auto i = 0; i < size; ++i) {
        if(i < countOfMines) {
            cells.push_back(new Cell(Cell::State::mine, this));
        }
        else {
            cells.push_back(new Cell(Cell::State::empty, this));
        }
    }

//...

return cells;

}

As you can see in the createCells method I set for each Cell the parent to this (which is Minefield);

Regardless of that the Cells don't get deleted from Qts Object Management. Why is that so? Because they are stored in the QVector ? Should I use QVector<QScopedPointer<Cell>> ?

JonB

@sandro4912

Regardless of that the Cells don't get deleted from Qts Object Management.

When is it that you expect what to get deleted? Your code shows nothing about deletion.

sandro4912

@JonB

IYoure right i forgot the most important part. When I delete Minefield i would have expected that all the Cells members in the QVector get deleted because i set there parent to Minefield

JonB

@sandro4912
I would too. But I don't use C++ or QVector. Do you need to do something like explicitly delete mCells in ~Minefield? Or, where does it say that QVector destructor is supposed to delete the items inserted into it?

SGaist

Hi,

How did you determine that the Cell objects are not deleted ?

fcarney

Have you tracked the deletion of the objects? Like put a destructor that prints out something when a cell is destroyed?

On a different note. You defined mCells as a private variable. Why not add the cells to that object rather than copying them after creating them locally?

sandro4912

@fcarney

Good suggestion. I changed the method to populate mCells in the constructor directly.

To trace the delete issue i added a destructor in Cell:

Cell::~Cell()
{
    static auto count = 1;
    qDebug() << "destroyed:" << this << " " <<count;
    ++count;
}

Doing this directly in main it deletes all the cells it has

Minefield *m = new Minefield;
   delete m;

However my deletion works like this in the program:

    auto newMinefield = new Minefield{ width, height, countOfMines };
    auto returnItem = mBottomFrame->layout()->replaceWidget(
                mMinefield, newMinefield);

    if(!returnItem) {
        mBottomFrame->layout()->addWidget(newMinefield);
    }
    else {
        delete returnItem;
    }
    
    mMinefield = newMinefield;

I assumed returnItem represents the old Minefield object.

I added the delete in the else :

else {
       delete mMinefield;
       delete returnItem;
   }

Now it seems like its working. Then why returnItem is not the old Minefield ? Is there a better solution (Using a smart pointer maybe?) to not get into trouble like this?

mrjj

Hi
The function is
https://doc.qt.io/qt-5/qlayout.html#replaceWidget
which returns QLayoutItem *
so the returnItem is not the minefield
however, its in
https://doc.qt.io/qt-5/qlayoutitem.html#widget

so i guess that is what you see.
Also notice docs says
"The returned layout item is no longer owned by the layout and should be either deleted or inserted to another layout."
Meaning you should delete it as you now own it (and the widget)

A no, using smart pointers with layouts will make you cry and crash.

sandro4912

So this QLayoutItem returned is like the geometry of how the widget is placed in the Layout and not the widget itself?

mrjj

@sandro4912
yes its its internal class handling the widget.
However, it does also hold the widget and
you can delete both if you want as you own them.