0

I am going crazy about two issues I am having with my code.

I am trying to delete an element from my vector containing a list of objects.

//Remove Object
if (button2 == true)
{ 
    //go through objects and check collision
    for (std::vector<cOBJECT*>::size_type i = 0; i != GameObjects.size(); i++)
    {
        //Check for collision and delete object
        if (MouseRect(GameObjects[i]->getrect(), mx + offX, my + offY) == true)
        {
            //GameObjects[i]->~cOBJECT();
            delete GameObjects[i];
            GameObjects.erase(GameObjects.begin() + i);
        }
    }
} // if (button2 == true)

For some reasons I run into two issues.

1) Access violation reading location 0xFEEEFEEE.

It seems to somehow have an issues with me destroying the texture. If I take out the "delete ...." and replace it with the destructor of the object instead, it works fine.

2) Vector subscript out of range

So if I use the destuctor to pass the first problem. I run into the next one. Now even if I use "GameObjects.erase(GameObjects.begin());" I get the same error.

songyuanyao
  • 169,198
  • 16
  • 310
  • 405
cedifra
  • 95
  • 6
  • If you must use a pointer, use a smart pointer and remove both the `delete` and destructor call lines. – chris Mar 27 '14 at 03:40
  • Thanks! But shouldnt it work like this anyway? – cedifra Mar 27 '14 at 03:44
  • You should use `i < GameObjects.size()`. If you delete the last element, then i will actually be one more than GameObjects.size() on the next iteration, and it will continue into the loop. – happydave Mar 27 '14 at 03:45
  • 1
    Alternatively, and perhaps better aligned with best practices, `i != GameObjects.end()` is the likely intent (Note the use of the end() member function replacing size()). – CPlusPlus OOA and D Mar 27 '14 at 03:49
  • @happydave thanks man! I tried that and I still get the error. This might be a stupid question, but could my objects have different sizes? And that is why? – cedifra Mar 27 '14 at 03:51
  • @CPlusPlusOOAandD Thanks! I tried that but receive the following: Error 9 error C2678: binary '!=' : no operator found which takes a left-hand operand of type 'unsigned int' (or there is no acceptable conversion) – cedifra Mar 27 '14 at 03:52
  • Okay, to compare to end(), i has to be declared as `std::vector::iterator`. Then the body code has to be modified to correspond to the iterator and whether the element is being accessed (e.g. `(*i).` or `delete *i` in your case. The erase invocation has to be updated accordingly if `i` becomes an iterator. – CPlusPlus OOA and D Mar 27 '14 at 03:57
  • You don't want to loop on `i` in the first place. You want to use an iterator and either advance it with the erasure when your action condition is *true*: (`delete *it; it = GameObjects.erase(it);`) **or** advance it with an increment when the action condition is *false*: `++it`, but *not both*. Finally, change the for-condition to be `it != GameObjets.end()` and remove the increment step entirely (or better still, use a `while` loop). – WhozCraig Mar 27 '14 at 04:04
  • Regarding the description given above, [See Example Here](http://pastebin.com/NP5eyVNZ) – WhozCraig Mar 27 '14 at 04:16
  • @WhozCraig works like a charm! Thanks man.Just had to add the "::iterator" – cedifra Mar 27 '14 at 04:24
  • @user3466904 Excellent. sry I posted it broken. thems the breaks whe posting to paste bin. So long as you understand how it works. Spend some time studying both general iterator concepts ([see here](http://en.cppreference.com/w/cpp/iterator)) and your container's iterator methods. After awhile, it all sort of gels. – WhozCraig Mar 27 '14 at 04:34

1 Answers1

3

If you think carefully about what the operation you implemented does you will notice that when the i-th element matches you remove that element from the vector and the (i+1)-th element is moved to the i-th position, but at this point the end of the loop is reached and i is incremented, which means that you will not test the element that was in the (i+1) position originally (and is now in i-th position) and also that if the value of i is GameObjects.size() - 1 before the removal the variable i now has a value that is GameObjects.size()+1 and the loop won't terminate.

Regarding the issue with the delete, you should check you created the object. Unless it was allocated with new chances are that you should not call delete on the pointer.

David Rodríguez - dribeas
  • 204,818
  • 23
  • 294
  • 489
  • @user3466904 Essentially, the intended algorithm versus actual implementation behaviors have to be reviewed. – CPlusPlus OOA and D Mar 27 '14 at 04:01
  • I understand. That makes sense. Once I found the one object, now I added a break; which then gets out of the loop! THANKS! That works very well. The delete.... works well now too! – cedifra Mar 27 '14 at 04:08
  • Thanks everybody! I have learned a lot. I really appreciate it! – cedifra Mar 27 '14 at 04:09
  • @user3466904: Breaking out of the loop will not detect if there is more than one element that needs to be removed. If that is fine in your problem, go ahead, but if you want to remove multiple elements, you will need to rework the loop. Also consider using smart pointers instead of raw pointers to avoid having to manage the memory manually. – David Rodríguez - dribeas Mar 27 '14 at 04:10
  • for this, it's fine. But I will def. study what I have been told here! – cedifra Mar 27 '14 at 04:18