CODEWITHSUNDEEP
c++
jonnywalkerr
jonnywalkerr

Reputation: 157

Trouble erasing items through list while iterating

Okay, I have a STL list of references I am iterating through. This function has three equivalent parts. The function takes a wstring as a parameter, and runs the appropriate if statement. I have reduced the code to one if statement to try and get it working.

So, I check to see what has been passed in as an argument. I then check to see if the ClassItem is a certain type of animal. If it is, I check if it is hungry, and erase it from the list. I am just trying to avoid seg faults right now, and cannot seem to do it. 

    list<ClassItem *>::iterator i = Items.begin();

    while(i != Items.end())    
    {
        if(!AnimalType.compare(L"tiger"))
        {
           if((*i)->IsAnimalType(L"tiger"))
           {
              if((*i)->IsHungry())
              {
                  i = Items.erase(i);

              }
           }
           else
           {
              ++i;
           }
        }
      // I have tried removing this
      else
      {
        i++;
      }
    }

I was under the impression that the current iterator is invalidated when I call erase. So, if I erase an element, I return the next valid iterator. Where am I going wrong?

EDIT: Thank you for all the quick help. The problem has been fixed. I have made use phresnel's solution and it worked wonderfully.

Upvotes: 0

Views: 92

Answers (3)

juanchopanza
juanchopanza

Reputation: 227608

You are better off by using std::list::remove_if with a suitable predicate. This avoids the manual loop entirely, reducing scope for errors, and helping to either remove or at least you localise the source of the problem, since you can rely on this idiom being correct as long as your predicate is.

bool badAnimal(ClassItem * item) 
{
  // return true if animal is to be removed 
 }

Items.remove_if(badAnimal);

Upvotes: 3

Sebastian Mach
Sebastian Mach

Reputation: 39109

I see no potential for a segfault here. Anyways:

There are (IMHO) two possible problems:

if(!AnimalType.compare(L"tiger"))

This looks fishy. What is AnimalType? Do you really expect the value of if(!AnimalType.compare(L"tiger")) to change during iteration, if AnimalType itself does not?

In any case, it looks like a read, therefore shouldn't write. It looks constant, therefore shouldn't change.

Then:

       if((*i)->IsAnimalType(L"tiger"))
       {
          if((*i)->IsHungry())
          {
              i = Items.erase(i);
          }
          // NO ITERATION IN CASE OF NOT HUNGRY.
          // ONCE TRAPPED HERE, YOU HAVE AN INFINITE LOOP,
          // EXCEPT AnimalType.compare(L"tiger") DOES SOMETHING
          // NON-SANE.
       }
       else
       {
          ++i;
       }

this should better be:

       if((*i)->IsAnimalType(L"tiger") && (*i)->IsHungry())
       {
          i = Items.erase(i);
       }
       else
       {
          ++i;
       }

However, even better would be to use the standard algorithms for element removal.

Upvotes: 1

CS Pei
CS Pei

Reputation: 11047

you may want to add 

continue;

after your erasion.

Upvotes: 0

Related Questions