Problem with deleting items from QGraphicsScene

[Wojtek_SZ]

Hi,

I have a problem with deleting QGraphicsItems from a scene. Item (pointer) was created by "new" in widget.cpp so it should be destroyed with "delete" anywhere - in every class. I'm trying to delete item (this) in advance function (located i n paliwo class) with is responsible for animation of my obiect:

void paliwo::advance(int phase)
{
 
    if(warunek){
        QList<QGraphicsItem*>Lista;
        QGraphicsItem* temp;
        samolot *temp_1;
        Lista = scene()->items(Qt::AscendingOrder);
 
        for(int i = 0;i<Lista.size();++i)
            if(Lista.value(i)->zValue() == 4)
                temp = Lista.value(i);
        temp_1 = dynamic_cast <samolot*>(temp);
        temp_1->pobierz(true);
        Lista.clear();                                        
        Lista.~QList();
        warunek = false;
    }
 
    if (!phase)
        return;
    else
        setPos(this->x(),this->y()+3);
    if (pos().y() > 300){
        this->~paliwo();
    }
}
 
 
paliwo::~paliwo()
{
    scene()->removeItem(this);
    delete this;
}

void paliwo::advance(int phase)
{

    if(warunek){
        QList<QGraphicsItem*>Lista;
        QGraphicsItem* temp;
        samolot *temp_1;
        Lista = scene()->items(Qt::AscendingOrder);

        for(int i = 0;i<Lista.size();++i)
            if(Lista.value(i)->zValue() == 4)
                temp = Lista.value(i);
        temp_1 = dynamic_cast <samolot*>(temp);
        temp_1->pobierz(true);
        Lista.clear();                                        
        Lista.~QList();
        warunek = false;
    }

    if (!phase)
        return;
    else
        setPos(this->x(),this->y()+3);
    if (pos().y() > 300){
        this->~paliwo();
    }
}


paliwo::~paliwo()
{
    scene()->removeItem(this);
    delete this;
}

To copy to clipboard, switch view to plain text mode 

- connected with widget.cpp:

timer_piechota = new QTimer(this);
    connect(timer_piechota, SIGNAL(timeout()),scena,SLOT(advance()));
    connect(timer_piechota, SIGNAL(timeout()),scena,SLOT(update()));
    timer_piechota->start(10);

timer_piechota = new QTimer(this);
    connect(timer_piechota, SIGNAL(timeout()),scena,SLOT(advance()));
    connect(timer_piechota, SIGNAL(timeout()),scena,SLOT(update()));
    timer_piechota->start(10);

To copy to clipboard, switch view to plain text mode 

Object in advance function has parent, so as I guess it shoud be easly removable with "delete". When I'am destroying parent program runs normaly, but after calling destructor of item which I want to destroy program crashes. I don't know what to do
I don' t want to destroy his parent but for now its only way to destroy it. Please help.

[pan]

Calling "delete this;" will call the destructor function ~paliwo()

Perhaps you need to checkout what delete and a destructor do in C++

[amleto]

wow, what are you doing there??? directly calling the destructor??? calling 'delete this'. I think you need to do a bit more research on simple c++ tutorials etc


For a beginner, never call the destrucor directly. get rid of .~QList and all of instances of calling destructor.

[stampede]

I think you just want to remove the item from the scene if a condition is met, so don't call "delete this", simply hide() the item.
You can then test if its visible when you enter the "advance" method and do the calculations only if its not hidden.
Some additional comments:

if(warunek){
        QList<QGraphicsItem*>Lista;
        QGraphicsItem* temp;
        samolot *temp_1;
        Lista = scene()->items(Qt::AscendingOrder);
 
        for(int i = 0;i<Lista.size();++i)
            if(Lista.value(i)->zValue() == 4)
                temp = Lista.value(i);
        temp_1 = dynamic_cast <samolot*>(temp); // temp_1 can be NULL, in that case...
        temp_1->pobierz(true);      // you will have a crash here, you need to test the pointer before using it
        Lista.clear();     // this is not needed
        Lista.~QList();  // this is not needed too, because Lista will go out of scope soon
        warunek = false;
    }

if(warunek){
        QList<QGraphicsItem*>Lista;
        QGraphicsItem* temp;
        samolot *temp_1;
        Lista = scene()->items(Qt::AscendingOrder);
 
        for(int i = 0;i<Lista.size();++i)
            if(Lista.value(i)->zValue() == 4)
                temp = Lista.value(i);
        temp_1 = dynamic_cast <samolot*>(temp); // temp_1 can be NULL, in that case...
        temp_1->pobierz(true);      // you will have a crash here, you need to test the pointer before using it
        Lista.clear();     // this is not needed
        Lista.~QList();  // this is not needed too, because Lista will go out of scope soon
        warunek = false;
    }

To copy to clipboard, switch view to plain text mode 

What do you mean by calling "delete this" in destructor ?
Your problems are pure (basic) C++, I think you should begin with a C++ book or two.

[Wojtek_SZ]

Thanks for all answers. It seems I need to grab a book and study about memory licking and destructors.

I made some corrections:

void paliwo::advance(int phase)
{
 
    if(warunek){
        QList<QGraphicsItem*>Lista;
        QGraphicsItem* temp;
        samolot *temp_1;
        Lista = scene()->items(Qt::AscendingOrder);
 
        for(int i = 0;i<Lista.size();++i)
            if(Lista.value(i)->zValue() == 4)
                temp = Lista.value(i);
        temp_1 = dynamic_cast <samolot*>(temp);
        if(temp_1){
            temp_1->pobierz(true);
            warunek = false;
        }
        }
 
    if (!phase)
        return;
    else
        setPos(mapToParent(0,3));
    if (pos().y() > 400)
        scene()->removeItem(this);
}

void paliwo::advance(int phase)
{

    if(warunek){
        QList<QGraphicsItem*>Lista;
        QGraphicsItem* temp;
        samolot *temp_1;
        Lista = scene()->items(Qt::AscendingOrder);

        for(int i = 0;i<Lista.size();++i)
            if(Lista.value(i)->zValue() == 4)
                temp = Lista.value(i);
        temp_1 = dynamic_cast <samolot*>(temp);
        if(temp_1){
            temp_1->pobierz(true);
            warunek = false;
        }
        }

    if (!phase)
        return;
    else
        setPos(mapToParent(0,3));
    if (pos().y() > 400)
        scene()->removeItem(this);
}

To copy to clipboard, switch view to plain text mode 

I decided to use removeItem function. Is it propper in this case?
According to DOC removes the item and all its children from the scene.
Removing but not deleting so what's gonna happen with this item after calling this function?
Allocated memory for item will remain occupied? How does scene handle with items deleting?
When durring animation item reaches position on scene which is out of QgraphicsView display range, is it automatically deleted?

stampede wrote:

temp_1 can be NULL, in that case

Yes it can. But only in case of incompatibility - in my code class samolot inherits form QGraphicItem, so I think its ok -at least I guess so...
Once again thanks for all answers

[stampede]

But only in case of incompatibility - in my code class samolot inherits form QGraphicItem, so I think its ok -at least I guess so...

Without checking for NULL value, you will have a crash soon after you place an item that is not a "samolot" on the scene and give it the zValue = 4.

[amleto]

if you remove the item from a scene, you will need to 'delete' it. But this probably means that you need to refactor your code - It is doubtful that an item should be responsible for removing itself from a scene in the first place.

[Wojtek_SZ]

I think I get it.

stampede wrote:

Without checking for NULL value, you will have a crash soon after you place an item that is not a "samolot" on the scene and give it the zValue = 4.

Is my code is enough secured in order to avoid such thing (NULL)
I think I have here clear situation about this practically there is no way to temp_1 be a NULL

Can anyone answer on my previous questions?