Iterator not incrementable?

So I am trying to write up two versions of a function, that takes in a list of data structure of students(which consists of their name and grades), and filters out all the failing students, and returns me back the list with only passing students.

One method simply erases the student from the list.

Code:

---

students extract_fails(students& studs){
        students fail;

        for(students::iterator i = studs.begin(); i != studs.end(); i++){
                if(!fgrade(*i)){
                        fail.push_back(*i);
                        studs.erase(i);
                }
        }

        return fail;
}

---

The other pushes all passing students to the beginning and resizes the list to the amount of passing students. The one I just mentioned works, but the one I posted up there doesn't. I get back an error saying the iterator is not incrementable. Why is this? Am I improperly erasing the element from the list?

Quote:

---

Originally Posted by dnguyen1022

Am I improperly erasing the element from the list?

---

You probably are by failing to account for the invalidation of the iterator. What container are you using?

I don't know if there is an algorithm that both removes and moves the erased elements to a different container, but this could be a task for partition/stable_partition which separates the students within a container. You can then cut out the failed students and put them in a different container (with std::list splice can be used for that).

Code:

---

typedef std::list<Student> StudentList;
StudentList extract_failed(StudentList& students)
{
    StudentList failed_students;
    StudentList::iterator it = std::stable_partition(students.begin(), students.end(), passes);
    failed_students.splice(failed_students.end(), students, it, students.end());
    return failed_students;
}

---

If you want to go with a hand-rolled loop, you have to keep in mind that erase() (usually) returns the next valid iterator, and you should do ++iterator only in case you didn't erase the current item.

I am using a list for the container.

As for your suggestion anon with the loop, I tried decrementing the iterator everytime i erased the element from the list, but it gives me an error saying the iterator is not decrementable. Your splice suggestion works great, but I would like to get this method working too!

Quote:

---

Originally Posted by dnguyen1022

As for your suggestion anon with the loop, I tried decrementing the iterator everytime i erased the element from the list, but it gives me an error saying the iterator is not decrementable. Your splice suggestion works great, but I would like to get this method working too!

---

By the time to try and decrement the iterator, it is already invalid. The erase member function for sequential containers return the iterator after the end of the point of erasure, so you can assign the return value of erase to your iterator object:

Code:

---

for (students::iterator i = studs.begin(); i != studs.end();) {
    if (!fgrade(*i)) {
        fail.push_back(*i);
        i = studs.erase(i);
    } else {
        ++i;
    }
}

---

Thank you laserlight. it worked brilliantly!