How do I delete the current QTreeWidget element correctly?
-
Hi!
I'm trying to delete the current QTreeWidget element using the 'delete' command.void SelectPreset::on_actionRemove_preset_clicked() // Remove preset { QTreeWidgetItem *item = ui_selectpreset->treeWidget->currentItem(); delete item; }
It works, but I'm not sure it's the right way.
-
It does not,
QTreeWidget
will retain a dangling pointer.void SelectPreset::on_actionRemove_preset_clicked() // Remove preset { QTreeWidgetItem *item = ui_selectpreset->treeWidget->currentItem(); if(!item) return; QTreeWidgetItem *parentItem = item->parent(); Q_ASSERT(parentItem); QTreeWidgetItem *takenItem = parentItem->takeChild(parentItem->indexOfChild(item)); Q_ASSERT(takenItem==item); delete takenItem; }
-
Here's what works for me:
void SelectPreset::on_actionRemove_preset_clicked() // Remove preset { int index = ui_selectpreset->treeWidget->currentIndex().row(); if (index < 0) { std::cout << "Negative index..." << std::endl; return; }; QTreeWidgetItem *item = ui_selectpreset->treeWidget->currentItem(); bool par = item->parent(); if (par == true) { std::cout << "Item is child... " << std::endl; QMessageBox msgBox; msgBox.setStyleSheet("background-color: rgb(5, 30, 35);"); msgBox.setIcon(QMessageBox::Question); msgBox.setWindowTitle("Preset"); msgBox.setText("Delete?"); msgBox.setStandardButtons(QMessageBox::Yes | QMessageBox::Cancel); msgBox.setDefaultButton(QMessageBox::Cancel); int confirm = msgBox.exec(); if (confirm == QMessageBox::Yes) { item->parent()->removeChild(item); }; } else { std::cout << "Item is parent... " << std::endl; int count_child = item->childCount(); if (count_child == 0) { ui_selectpreset->treeWidget->takeTopLevelItem(index); }; }; }
-
@Helg1980
You code may "work" for you, but you are doing nothing about your question title, "How do I delete the current QTreeWidget element correctly".item->parent()->removeChild(item);
This *leaks":
Removes the given item indicated by child. The removed item will not be deleted.
ui_selectpreset->treeWidget->takeTopLevelItem(index);
Removes the top-level item at the given index in the tree and returns it, otherwise returns nullptr;
Although the docs are not explicit, I imagine this leaks in a similar fashion.
Run your program under, say,
valgrind
and see what you get.On a separate note, it might be just me but I have never seen people write
bool par = item->parent();
assigning a pointer to a
bool
without any explicit cast. Do you not find, say,bool par = item->parent() != nullptr;
cleaner/nicer? -
Here's what I got after the adjustments:
void SelectPreset::on_actionRemove_preset_clicked() // Remove preset { int index = ui_selectpreset->treeWidget->currentIndex().row(); if (index < 0) { std::cout << "Negative index..." << std::endl; return; }; QTreeWidgetItem *item = ui_selectpreset->treeWidget->currentItem(); QTreeWidgetItem *parentItem = item->parent(); //Q_ASSERT(parentItem); if (parentItem != nullptr) { std::cout << "Item is child... " << std::endl; QMessageBox msgBox; msgBox.setStyleSheet("background-color: rgb(5, 30, 35);"); msgBox.setIcon(QMessageBox::Question); msgBox.setWindowTitle("Preset"); msgBox.setText("Delete?"); msgBox.setStandardButtons(QMessageBox::Yes | QMessageBox::Cancel); msgBox.setDefaultButton(QMessageBox::Cancel); int confirm = msgBox.exec(); if (confirm == QMessageBox::Yes) { QTreeWidgetItem *takenItem = parentItem->takeChild(parentItem->indexOfChild(item)); //Q_ASSERT(takenItem==item); delete takenItem; }; } else { std::cout << "Item is parent... " << std::endl; std::cout << "Index top level: " << index << std::endl; int count_child = item->childCount(); if (count_child == 0) { ui_selectpreset->treeWidget->takeTopLevelItem(index); } else { QMessageBox msgBox; msgBox.setStyleSheet("background-color: rgb(5, 30, 35);"); msgBox.setIcon(QMessageBox::Information); msgBox.setWindowTitle("Preset"); msgBox.setText("Delete presets first! "); msgBox.setStandardButtons(QMessageBox::Ok); msgBox.exec(); }; }; }
-
@Helg1980 said in How do I delete the current QTreeWidget element correctly?:
delete takenItem;
That is better.
//Q_ASSERT(takenItem==item);
I would have thought that should hold, is there a reason you commented out?
ui_selectpreset->treeWidget->takeTopLevelItem(index);
I still think that needs
delete
-ing? Check withvalgrind
or other memory leak checker? -
@Helg1980 said in How do I delete the current QTreeWidget element correctly?:
//Q_ASSERT(parentItem);
Q_ASSERT(parentItem);
For some reason, I have a crash with this command, and there is no decryption of the ASSERT error, despite the fact that I added #include <QtGlobal>, so I commented out all the lines with Q_ASSERT.
Please tell me an alternative to the command
ui_selectpreset->treeWidget->takeTopLevelItem(index);
to avoid leakage.
-
Q_ASSERT(parentItem);
For some reason, I have a crash with this command, and there is no decryption of the ASSERT error
When compiled for debug, failure on this line should simply show you which line (probably including the word
parentItem
) it failed on. That's whatQ_ASSERT()
does.Q_ASSERT(parentItem);
in your code will correctly assert, when the item being deleted has no parent (i.e. top-level). Therefore your code should indeed not have that line. But the line I said it could have is the commented-out//Q_ASSERT(takenItem==item);
, which should succeed logic-wise. This is a detail.Please tell me an alternative to the command
ui_selectpreset->treeWidget->takeTopLevelItem(index);
to avoid leakage.
You wouldn't want me to tell you, because you know this. When deleting a non-top-level element you already have
QTreeWidgetItem *takenItem = parentItem->takeChild(parentItem->indexOfChild(item)); Q_ASSERT(takenItem==item); delete takenItem;
So what do you think you have correspondingly in the
takeTopLevelItem()
place? -
I replaced the line by analogy
ui_selectpreset->treeWidget->takeTopLevelItem(index);
with the following lines:
QTreeWidgetItem *takenItem = ui_selectpreset->treeWidget->takeTopLevelItem(ui_selectpreset->treeWidget->indexOfTopLevelItem(item)); Q_ASSERT(takenItem==item); delete takenItem;
(The only thing is that I couldn't start Q_ASSERT, I already turned everything on and still it doesn't work for me.)
-
@Helg1980 said in How do I delete the current QTreeWidget element correctly?:
The only thing is that I couldn't start Q_ASSERT
I have no idea what you mean by this. What does "start" mean here?
Q_ASSERT()
is a macro, producing code during compilation.and still it doesn't work for me.
In what way "work" or "doesn't work"?
BTW, your Build Settings are not relevant to this.
-
@Helg1980
That's not actually any kind of compiler command, just a make-type one, but never mind.I really don't know. If the first line of your program is a
Q_ASSERT(false)
it should produce an error (when run) if compiled for Debug. And if you run your application from the debugger it should cause that to break on the offending line. If it'sQ_ASSERT(true)
it should do precisely nothing. You might test with these two.P.S.
Where are you looking for the message to appear? If running from Qt Creator I think it would go to the Application Output pane. -
@Helg1980 said in How do I delete the current QTreeWidget element correctly?:
Q_ASSERT(parentItem);
That's the point, that in Q_ASSERT(parentItem), the value 'parrentItem' is false, but Q_ASSERT does not give a message about this, but simply crashes the program;
-
Yes, I work in QtCreator.
Here is my Application Output:18:31:38: Starting /run/media/helg/GOODRAM-SDB/QtCreatorProjects/CineEncoder/build-cine_encoder-Desktop-Debug/cine_encoder ... 18:31:53: The program has unexpectedly finished. 18:31:53: The process was ended forcefully. 18:31:53: /run/media/helg/GOODRAM-SDB/QtCreatorProjects/CineEncoder/build-cine_encoder-Desktop-Debug/cine_encoder crashed.
-
@Helg1980
OK, I don't know what you are doing, and what your program is doing.It is time now to use the debugger to put a breakpoint in your program, and step through offending code. Besides, I don't know how you are getting a "crashed" without the debugger catching it and showing you a stack trace back to the faulting line. If you are not familiar with using the debugger it's time you were, as you can resolve these issues very simply by using it!
-
@Helg1980
Start the debugger. If you have a debugger in the background it will tell you where the assertion occurred.Q_ASSERT
andQ_ASSUME
are used to test implicit assumptions you make in your code.Looking at the sources
QTreeWidget
it looked likeitem->parent()
for a top level item would return the invisible root item. I tested this implicit assumption withQ_ASSERT
and it turns out I was wrong (but at least we know why)void SelectPreset::on_actionRemove_preset_clicked() // Remove preset { QTreeWidgetItem *item = ui_selectpreset->treeWidget->currentItem(); if(!item) return; QTreeWidgetItem *parentItem = item->parent(); QTreeWidgetItem *takenItem = nullptr; if(parentItem) takenItem = parentItem->takeChild(parentItem->indexOfChild(item)); else takenItem = ui_selectpreset->treeWidget->takeTopLevelItem(ui_selectpreset->treeWidget->indexOfTopLevelItem(item)); Q_ASSERT(takenItem==item); delete takenItem; }