2

I currently have 3 classes that are public QWidgets. -> x, y ,z are all qwidgets.

In my mainwindow I have pointers to x, y, z.

So the members:

X* m_x;
Y* m_y;
Z* m_z;

A member function:

void MainWindow::deleteScreen(QWidget** widget)
{
    if(widget != NULL)
    {
        delete widget;
        widget = NULL;
    }
}

called as: deleteScreen(&m_x); -> causes invalid conversion.

If I changed the deleteScreen param to QWidget* widget and call as deleteScreen(m_x) it will delete the memory but it won't set m_x to NULL. (only the local variable, widget)

-> Is there any way to make the deleteScreen function delete the given widget AND put the value of the member variable on NULL?

Thank you!

Stano
  • 8,749
  • 6
  • 30
  • 44
  • 1
    if ( widget != null && *widget != null ) {`delete *widget; *widget = NULL;}` – Alex F Sep 01 '13 at 11:26
  • 1
    Are you sure that QWidgets must be deleted? AFAIK, Qt implements some kind of garbage collection. – Alex F Sep 01 '13 at 11:27
  • @AlexFarber I did not know that Qt had garbage collection. But if it didn't how would your solution work? I tried something like that but I can't call the function with deleteScreen(&m_x) -> invalid conversion. – secondVISION Sep 01 '13 at 11:39
  • 2
    Qt does not have garbage collection but QObject is an implementation of the Composite Pattern so all child objects are deleted when the parent is deleted. – RobbieE Sep 01 '13 at 11:44
  • 2
    May I add the comment, that it is not necessary to test for a NULL pointer before deleting it? Deleting a NULL pointer is actually well defined, and does.... nothing. So drop the if(widget != NULL). It is superfluous and makes your code look less professional. ;-) – Greenflow Sep 01 '13 at 11:45
  • 1
    If you really want to 'delete' it and assign a NULL value, why not use reference. void MainWindow::deleteScreen(QWidget* & widget) – Yuan Sep 01 '13 at 11:51

2 Answers2

3

Actually, a double pointer to a base class cannot be used as a double pointer to a derived class; so you probably have to resort to templates:

template<typename T>
void deleteAndCleanup(T*& ptr)
{
    delete ptr;
    ptr = NULL;
}

// called like this:
deleteAndCleanup(m_x);
Community
  • 1
  • 1
Matteo Italia
  • 123,740
  • 17
  • 206
  • 299
  • You don't need to check for NULL in the version that takes a reference. Calling delete on a NULL pointer is perfectly fine. (You still need to check for NULL in the version that takes a pointer of course, since it gets dereferenced.) – Nikos C. Sep 01 '13 at 11:54
  • @NikosC.: the NULL check isn't needed in any of them; if the double-pointer is NULL there's a serious logic flaw in the caller, so a segfault is much deserved. – Matteo Italia Sep 01 '13 at 11:56
  • Hey, I tried the first solution but that gives me the invalid conversion: error: invalid conversion from 'X**' to 'QWidget**' [-fpermissive] – secondVISION Sep 01 '13 at 11:56
  • The second solution also gives an error: error: no matching function for call to 'MainWindow::deleteScreen(X*&)' – secondVISION Sep 01 '13 at 11:58
  • @secondVISION: the second solution gives you that error because you haven't updated the declaration inside the `class` block, but it suffers the same problems as the first one (you can't use a reference to a pointer to a base class to pass a pointer to a derived class); see updated answer for an alternative solution. – Matteo Italia Sep 01 '13 at 12:02
  • @MatteoItalia Alright, thank you! The new solution works. I'll read that other question for some information. – secondVISION Sep 01 '13 at 12:06
3

The problem here is not so much that you want to delete the QWidgets but that you are using the pointers to them to determine whether they have been deleted or not.

Qt has a solution for this. For QObject classes, you can use the QPointer class as a guard. It is aware of whether the QObject it contains has been deleted or not by being connected internally to the object's deletion.

Try the following as an alternate to your code:

QPointer<X> m_x;
QPointer<Y> m_y;
QPointer<Z> m_z;


template<typename T>
void MainWindow::deleteScreen(QPointer<T> &widget)
{
    if(!widget.isNull() && qobject_cast<QWidget>(widget) != 0)
    {
        widget->deleteLater();
    }
}
RobbieE
  • 4,280
  • 3
  • 22
  • 36
  • I think the `&& qobject_cast(widget) != 0` part is not really useful alone. If you want to keep it, you should probably add `&& widget->parent() == this` or something, to make it actually work as some kind of safety check. – hyde Sep 01 '13 at 13:41
  • The reason I put that check in was merely to make the functionality similar to secondVISION's original code, that only `QWidget` items should be handled by the function. This restriction is lost by making it a template function since `QPointer` can hold any `QObject`, not only `QWidget`s – RobbieE Sep 02 '13 at 10:41