Removing items from boost ptr_list

[beany37]

I am using boost::ptr_vector to keep a list of pointers so that when the list is destructed all the pointers get deleted automatically. However, sometimes when I add an object to the list I keep a pointer to that object (although I still consider the ptr_list to have ownership) so that I can access it directly and also so that I can later remove it from the list.

The problem is boost::ptr_list doesn't have the remove function like std:list does. There doesn't seem to be anyway to remove an item based on its address. The only solution I can see is to do something like:

typedef boost::ptr_list<Foo> Foo_list;
Foo_list foo_list;
Foo_list::iterator iter;

foo_list.push_front (new Foo);
iter = foo_list.begin();
...
foo_list.erase(iter);

But this seems ugly and surely not what iterators are meant to be used for.

Am I missing something?

[firedraco]

Instead of keeping a pointer, try keeping an iterator to the element.

[jsmith]

If it's just a matter of scope inside a single function, then go with what firedraco said.

If not, then from a design standpoint, either you want the container to wholly own the object or you don't. By holding on to a pointer outside the container, you are saying you don't want the container to have the sole responsibility for owning object. In that case, perhaps using a std::list<> of boost::shared_ptr<> while keeping a boost::weak_ptr<> outside the container is a more appropriate model of the ownership semantics.

[beany37]

If it's just a matter of scope inside a single function, then go with what firedraco said.

The pointer/iterator needs to be stored for much longer than the scope of a single function.

In that case, perhaps using a std::list<> of boost::shared_ptr<> while keeping a boost::weak_ptr<> outside the container is a more appropriate model of the ownership semantics.

This doesn't help me remove the item from the list when I need to. I'd still have to iterate through the whole list to find the item to remove.

I'm beginning to think keeping an iterator really is the best thing to do. It allows me to remove the item later with constant complexity (it's a list), and none of the list operations invalidate list iterators (unless of course they operate on this particular item, which I know they won't).

So I can't see any reason not to do this...

[jsmith]

Ok, if you are concerned about lookup efficiency, then perhaps a map would be better?

[beany37]

But seriously, is there any good reason not to just store the iterator? That way I get constant time removal, where even with a map it's log (n). There seems to be a taboo about keeping iterators beyond function scope.

To be honest, the efficiency isn't *that* important to me. But I just don't like the idea of iterating or doing a map lookup when I could just store the iterator and get constant time removal...

[jsmith]

The reason why it isn't recommended to keep iterators beyond function scope is exactly the invalidating iterator reason. std::list eliminates that problem, but at the same time, continuing with that design locks you into a container that doesn't invalidate iterators on insert or remove (without a potential major rework). If you are confident that you'll never want the container to be a std::vector or std::deque, then your solution will work fine.

(When I'm designing something, I try to make my code as generic as possible while meeting the requirements. And I have on many occasions in the past designed entire programs using one container, then months later when a new feature came it, completely replaced the container with a different one in exactly one line of code -- changing a typedef. Of course, my tendencies are just that -- my tendencies.)

[beany37]

That's an interesting point, I hadn't considered the possibility that I'd want to change container type in the future. I think on this project I'll stick with the iterator design as I'm quite confident a list will continue to serve my purposes, but I will consider this point in future.