C++ deleting from a vector of objects

Question

I have a few vectors that contain objects and I have a nested series of loops to get to the object I want

if (_dir->Instance()->isDebug())
{
    Utils::LogTextWithInt("growing timers size: ", _glo->Instance()->getGrowingTimers().size());
    for (int i=0; i < _glo->Instance()->getGrowingTimers().size(); i++)
    {
         GrowingTimer _growingTimer = _glo->Instance()->getGrowingTimers().at(i);

        std::cout << "growing timer Field id and plant id: " << _growingTimer.getFieldID() << " - "
                    << _growingTimer.getPlantID() << std::endl;
    }
}

std::vector<GrowingTimer>::iterator _gtIterB = _glo->Instance()->getGrowingTimers().begin();

for (_gtIterB; _gtIterB != _glo->Instance()->getGrowingTimers().end(); ++_gtIterB)
{       
    for (std::vector<Field>::iterator _fIterB = _glo->Instance()->getFields().begin();
         _fIterB != _glo->Instance()->getFields().end(); ++_fIterB)
    {          
        if (_gtIterB->getFieldID() == _fIterB->getFieldNumber())
        {
            for (std::vector<Plant>::iterator _pIterB = _fIterB->getPlants().begin();
                 _pIterB != _fIterB->getPlants().end(); )
            {
                if (_gtIterB->getPlantID() == _pIterB->getPlantID())
                {
                    Utils::LogText("gt and plant ID's match");
                    Utils::LogTextWithInt("Plant ID after matching: ", _pIterB->getPlantID());

                    // Wiggle our plant.
                    Utils::wiggleNode(_pIterB->getPlantSprite(), 10.0f, 5.0f);

                    _pIterB->setPlantStoppedGrowing(true);

                    _gtIterB = _glo->Instance()->getGrowingTimers().erase(_gtIterB);

                    ++_pIterB;
                }
                else
                {
                   ++_pIterB;
                }

            }
        }
    }
}

The output is:

growing timers size:  2
growing timer Field id and plant id: 7620 - -2130819608
growing timer Field id and plant id: 7620 - -2130802800
gt field id:  7620 
gt plant id:  -2130819608
gt and plant ID's match
Plant ID after matching:  -2130819608
deleted
gt and plant ID's match
Plant ID after matching:  -2130802800
deleted
gt field id:  7620
gt plant id:  -2130802800
gt and plant ID's match
Plant ID after matching:  -2130802800

I am crashing after all of this:

0x209eff0:  addb   %ah, %gs:115(%ecx,%ebp,2)
0x209eff5:  jo     0x209f063                 ; "nitWithCondition:"
0x209eff7:  popal  
0x209eff8:  jns    0x209f048                 ; "'v'"
0x209effa:  popal  
0x209effb:  insl   
0x209effc:  incl   %esi
0x209effe:  outsl  
0x209efff:  jb     0x209f04c                 ; "ckBeforeDate:"
0x209f001:  jns    0x209f03e                 ; "Name:"
0x209f004:  jbe    0x209f067                 ; "ithCondition:"
0x209f006:  insb   
0x209f007:  jne    0x209f06e                 ; "ition:"
0x209f009:  cmpb   (%eax), %al
0x209f00b:  popl   %edi
0x209f00c:  jo     0x209f080                 ; "lSinceNow"

Basically now that I have a match and I act upon it I want to delete the position in _gIterB as it is no longer needed. This vector keeps getting added to as a "tracker" if you will of timed events that are running so that when the timers are finished we know what object the timer was for and can get back to it to do the next steps.

EDIT: The vector of GrowingTimer store the fieldID and the PlantID that is "growing" so we can get back to it.

The vector of Fields stores all the fields the user has and as a member each Field Object has a vector of the plants in the field. 8 plants to 1 field.

each field the user creates has a unique ID, the output below is there is one field with 2 plants in it.

Because vectors use an array as their underlying storage, erasing elements in positions other than the vector end causes the container to relocate all the elements after the segment erased to their new positions. So iterators are invalid after erasing an element. What I generally do is to save the index of the thing to be deleted into a separate vector and then run through and erase those elements after the kloop to find them has finished. — Jerry Jeremiah, Aug 28 '13 at 00:21
can several fields have the same ID? Because if not this can be simplified — Mooing Duck, Aug 28 '13 at 00:24
@JerryJeremiah - so you are saying to make a temp vector that contains the iterators of the ones to be deleted and periodically run through that vector, get the ones to be deleted and execute an erase() on the the GrowingTimer for those iterators? — Jasmine, Aug 28 '13 at 00:31

Mooing Duck · Answer 1 · 2013-08-28T00:44:10.467

0

for (auto _gtIterB=grow_timers.begin(); _gtIterB!=grow_timers.end(); ++_gtIterB) {     
    ...
    _gtIterB = _glo->Instance()->getGrowingTimers().erase(_gtIterB);
    .... 
}

So basically, there's two problems here.

when you erase _gtIterB, you assign it the position of the next grow timer. This means you've skipped comparing this growtimer to many plants. I presume this is bad, but probably isn't causing your crash.
If you erase _gtIterB when it's the last valid growtimer it's set to the end iterator, then when you get to the next part of the for loop, you have ++_gtIterB again, causing it to go past the end, causing who knows what to happen. This is definitely bad.

I'm not certain either of these are causing your crash.

But here's my interpretation of your code that's faster and not deeply nested: http://ideone.com/zcfnv8

edited Aug 28 '13 at 00:44

answered Aug 28 '13 at 00:23

Mooing Duck

64,318
19
100
158

I was trying what is outlined here: http://stackoverflow.com/questions/991335/how-to-erase-delete-pointers-to-objects-stored-in-a-vector – Jasmine Aug 28 '13 at 00:32
@Jason: You mean the part that says `for (it=Entities.begin(); it!=Entities.end(); )` instead of what you have? – Mooing Duck Aug 28 '13 at 00:33
they also seem to increment the iterator, which I thought was a bit weird as you can do that as part of the for loop. – Jasmine Aug 28 '13 at 00:50
1

@Jason: Actually, that's exactly the problem. If you have a call to erase, you _cannot increment the iterator as part of the for loop_. That is the cause of your crash. Look at the code you linked. It erases, _or_ it advances the iterator. Do not do both. – Mooing Duck Aug 28 '13 at 00:57
But I am deleting an element from a different vector that what the for loop is using. I am deleting out of the main For loop of GrowingTimers... – Jasmine Aug 28 '13 at 15:16
You have three for loops. One of them is iterating over the grow timers, and your innermost loop is erasing the current grow timer inside that outer loop. The fact that there's other for loops is completely irrelevent. – Mooing Duck Aug 28 '13 at 16:25
Sorry, explain why I would not iterate the inner most for loop after deleting from the outer most. I just dont see why you would not iter the inner after deleting from the outer. – Jasmine Aug 28 '13 at 16:41
@Jason: When you delete from the outermost, `_gtIterB` now points at the _next_ timer, or sometimes no timer at all. If it points at the _next_ timer, that means you've failed to compare _that_ timer to many plants and fields. If it points to no timer at all, the next time it hits `_gtIterB->getPlantID() == _pIterB->getPlantID()` or similar, it will crash. – Mooing Duck Aug 28 '13 at 16:46

C++ deleting from a vector of objects

1 Answers1