CODEWITHSUNDEEP
c++classvector
frendall
frendall

Reputation: 63

c++ vector object .erase

I have been struggling to put a vector object into a project im doing I have read what little i could find about doing this and decided to give it a go. 

      std::vector<BrickFalling> fell;
      BrickFalling *f1;

I created the vector. This next piece works fine until i get to the erase section. 

if(brickFall == true){

f1 = new BrickFalling;

f1->getBrickXY(brickfallx,brickfally);

fell.push_back(*f1);
brickFall = false;
}
// Now setup an iterator loop through the vector

vector<BrickFalling>::iterator it;

for( it = fell.begin(); it != fell.end(); ++it ) {

   // For each BrickFalling, print out their info

   it->printBrickFallingInfo(brick,window,deadBrick);

   //This is the part im doing wrong /////
   if(deadBrick == true)// if dead brick erase
   {
       BrickFalling[it].erase;//not sure what im supposed to be doing here
       deadBrick = false;
   }
}

Upvotes: 0

Views: 276

Answers (2)

PaulMcKenzie
PaulMcKenzie

Reputation: 35440

You can totally avoid the issue by using std::remove_if along with vector::erase.

auto it = 
   std::remove_if(fell.begin(), fell.end(), [&](BrickFalling& b) 
              { bool deadBrick = false;      
                b.printBrickFallingInfo(brick,window,deadBrick);
                return deadBrick; });

   fell.erase(it, fell.end());

This avoids the hand-writing of the loop.

In general, you should strive to write erasure loops for sequence containers in this fashion. The reason is that it is very easy to get into the "invalid iterator" scenario when writing the loop yourself, i.e. not remembering to reseat your looping iterator each time an erase is done.

The only issue with your code which I do not know about is the printBrickFallingInfo function. If it throws an exception, you may introduce a bug during the erasure process. In that case, you may want to protect the call with a try/catch block to ensure you don't leave the function block too early.

Edit:

As the comment stated, your print... function could be doing too much work just to determine if a brick is falling. If you really are attempting to print stuff and do even more things that may cause some sort of side-effect, another approach similar in nature would be to use std::stable_partition.

With std::stable_partition you can "put on hold" the erasure and just move the elements to be erased at one position in the container (either at the beginning or at the end) all without invalidating those items. That's the main difference -- with std::stable_partition, all you would be doing is move the items to be processed, but the items after movement are still valid. Not so with std::remove and std::remove_if -- moved items are just invalid and any attempt to use those items as if they are still valid is undefined behavior.

auto it = 
   std::stable_partition(fell.begin(), fell.end(), [&](BrickFalling& b) 
              { bool deadBrick = false;      
                b.printBrickFallingInfo(brick,window,deadBrick);
                return deadBrick; });

   // if you need to do something with the moved items besides
   // erasing them, you can do so.  The moved items start from
   // fell.begin() up to the iterator it.
   //...
   //...
   // Now we erase the items since we're done with them
   fell.erase(fell.begin(), it);

The difference here is that the items we will eventually erase will lie to the left of the partitioning iterator it, so our erase() call will remove the items starting from the beginning. In addition to that, the items are still perfectly valid entries, so you can work with them in any way you wish before you finally erase them.

Upvotes: 4

MicroVirus
MicroVirus

Reputation: 5477

The other answer detailing the use of remove_if should be used whenever possible. If, however, your situations does not allow you to write your code using remove_if, which can happen in more complicated situations, you can use the following:

You can use vector::erase with an iterator to remove the element at that spot. The iterator used is then invalidated. erase returns a new iterator that points to the next element, so you can use that iterator to continue.

What you end up with is a loop like:

for( it = fell.begin(); it != fell.end(); /* iterator updated in loop */ )
{
     if (shouldDelete)
         it = fell.erase(it);
     else
         ++it;
}

Upvotes: 3

Related Questions