5 replies to this topic

[EpicCupcakes]

I've been having a small problem with containers in C++, mainly the "list" container. I've been trying to cycle through a list using an iterator and delete items with a certain criteria, but I've been having problems using the list's "erase" and "remove" functions to delete those items from the list. I don't know if I'm misusing the functions or if the list is simply not maid for jobs like this.

Here's the code I've been running:

#include <iostream>
#include <list>
using namespace std;
class SayProcess
{
    public:
    SayProcess(){}
    void Say()
    {
	    cout << "Say" << endl;
    }
    ~SayProcess()
    {
	    cout << "Deleted" << endl;
    }
};
int main()
{
    SayProcess s1;
    SayProcess s2;
    SayProcess s3;
    list<SayProcess> List;
    List.push_back(s1);
    List.push_back(s2);
    List.push_back(s3);
    list<SayProcess>::iterator it = List.begin();
    for(; it != List.end(); it++)
    {
	    (*it).Say();
	    List.erase(it);
    }
    cout << "List Completely Deleted!" << endl;
    char x;
    cin >> x;
    return 0;
}

This is a simplified version of the code that is broken, but it has the same problems. I was hoping to simply have the loop print out each Process's message and then erase the Process from the list, but it's not working the way I had intended.

When run, this code will repeatedly print the SayProcess's two messages, the "Say" function's message and the destructor's message until infinity. The fact that it is repeatedly calling the destructor confuses me. Is it copying the item somehow?

I feel as if the problem is in my understanding of how either lists or iterators work, but I don't know enough about their design to tell why, and MSDN has been unhelpful. Can anyone help out by telling me what I'm doing wrong?

[SiCrane]

Standard library containers store copies of the objects you insert them, not the original objects. Also, calling erase() on an iterator invalidates that iterator, but returns an iterator to the next object in the container. For standard library sequence containers you would want to use a loop like the following:

    list<SayProcess>::iterator it = List.begin();
    for(; it != List.end();)
    {
            it->Say();
            it = List.erase(it);
    }

[dimitri.adamou]

If your compiler supports Lambda expressions try this one out

auto it = remove_if(List.begin(), List.end(), [](const SayProcess& lp) { lp.Say(); return true; });

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

Edited by dimitri.adamou, 12 August 2012 - 03:05 AM.

[EpicCupcakes]

Standard library containers store copies of the objects you insert them, not the original objects. Also, calling erase() on an iterator invalidates that iterator, but returns an iterator to the next object in the container. For standard library sequence containers you would want to use a loop like the following:

	list<SayProcess>::iterator it = List.begin();
	for(; it != List.end();)
	{
			it->Say();
			it = List.erase(it);
	}

Thank you very much for this response. I didn't know that erase worked on iterators like that. My code works like a charm now! Thanks again.

[pekarn]

If your compiler supports Lambda expressions try this one out

auto it = remove_if(List.begin(), List.end(), [](const SayProcess& lp) { lp.Say(); return true; });

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

Actually std::list has a member function remove_if that will do this more efficiently (because it doesn't have to move the elements).
So the above code would be replaced by
[source lang="cpp"]List.remove_if( [](const SayProcess& lp) -> bool { lp.Say(); return true; } );[/source]
This is only for list though. If your container is std::vector for example you have to use the erase-remove idiom demonstrated by dimitri.adamou.

Also I believe lambda functions can only leave out the return type if the function consists of only a single return statement. I added the return type in my code above.

[dimitri.adamou]

Thanks for that, I didn't know List had its own remove_if