views:

215

answers:

7

Suppose I have a std::vector<Obj *> objs (for performance reasons I have pointers not actual Objs).

I populate it with obj.push_back(new Obj(...)); repeatedly.

After I am done, I have to delete the pushed-back elements. One way is to do this:

for (std::vector<Obj *>::iterator it = objs.begin(); it != objs.end(); ++it) {
    delete *it;
}

However, I am interested if I can use for_each algorithm to do the same:

#include <algorithm>
...
for_each(objs.begin(), objs.end(), delete);

What do you think?

Thanks, Boda Cydo.

+4  A: 

Yes, but you need a functor:

struct delete_ptr
{
    template <typename T>
    void operator()(T* pPtr)
    {
        delete pPtr;
    }
};

std::for_each(objs.begin(), objs.end(), delete_ptr());

In C++0x, lambda's help you make functors in-place:

std::for_each(objs.begin(), objs.end(), [](Obj* pPtr){ delete pPtr; });

However, this is dangerous, in the face of exceptions. sbi has shown a solution.

GMan
Mr. GMan, can you please explain the use of functor. Particularly, why plain `delete` won't work? Thanks.
bodacydo
@bodacydo: The reason `delete` won't work on the language level is it's a keyword, and you can't pass keywords into functions. You can't call it with `delete` anymore than you could call it with `catch` or `static_cast`. (Functions need to be passed variables. Functors and function pointers are such.)
GMan
Ah, I was just about to ask why nobody showed how to do this with lambda, when I saw that I had missed out yours. `:)`
sbi
+6  A: 

Your problem is that delete is not a function, but rather a keyword and as such you can't take it's address.

In C++0x, there will be a std::default_delete class (used by std::unique_ptr), which you could use, or - as everyone's saying - writing one yourself would be trivial (the standard one also raises a compile error, if you try to delete an incomplete type).

#include <vector>
#include <algorithm>
#include <memory>

int main()
{
    std::vector<int*> vec;
    std::for_each(vec.begin(), vec.end(), std::default_delete<int>());
}
UncleBens
I suppose `operator+` (which I believe could be used with `std::foreach()`) then isn't a keyword, but the name of a function whose name happens to consist of a keyword and an operator?
sbi
@sbi: How would that look like? As far as I know, if you want to use `operator+`, then the function object you need is called `std::plus`. (And yes, `operatorX` is a function, except for builtin types.)
UncleBens
@UncleBens: Are you saying we can't you take the address of a function that happens to be an operator?
sbi
GMan
@sbi: My point is that when you do something like `std::foreach(a, b, static_cast<void (*)(void*)>(` then no destructors will be called. `operator delete` - overloaded or not - does not invoke destructors, it only deals with raw memory. - If you know a way how to take the address of delete function, so the result is equivalent to `delete p;`, please show.
UncleBens
@UncleBens: I wasn't referring to `delete` especially, but to keywords in general. And I wasn't disputing what you said, but merely trying to get a confirmation that I agreed with you for the right reason. `:)` (@GMan seems to agree that one can take the address of an `operator+()` function. But this would still not be passing a keyword to `std::foreach()`, because the keyword is `operator`, not `operator+`.)
sbi
+3  A: 

While you can do this (GMan has shown a solution), having containers with naked pointers to owned resources is a strong code smell. For example, in this code:

  void foo()
  {
    std::vector<Obj *> bar;
    fill(bar);
    use(bar);
    std::for_each(objs.begin(), objs.end(), delete_ptr()); // as GMan suggests
  }

if use() throws, you'll leak objects.

So it's better to use smart pointers for this:

std::vector< std::shared_ptr<Obj> > bar;
sbi
I win. :​​​​​​) But I like your format, let's form a cyclic dependency.
GMan
@GMan: Garrghh! You have me stuck in a loop. Grrr.
Charles Bailey
I'd be interested in why this was down-voted. What's wrong with it?
sbi
@GMan: I'm confused. What did you win??
sbi
@sbi: I had your answer in mine, before you posted your answer. So I "won [the race]". But I liked yours more, so I looped 'em together.
GMan
@GMan: Oh, I hadn't even seen that. That looping is a wicked thing. (Like googling for "recursion".)
sbi
+1  A: 

for_each needs a function pointer or function object. For memory deallocation you could try &::operator delete, which would take the address of the function that deallocates memory. However, when you use the delete statement the compiler calls the destructor before calling operator delete(void*) so cleanup is actually not part of the operator delete(void*) function.

Use a functor ala GMan's answer.

Ben Voigt
bodacydo
@bodacydo: You're correct, this won't call the destructor. This can only be used for deallocating blocks of raw memory, not for deleting objects.
Mike Seymour
+1  A: 

Not exactly; for_each requires a function or object that can be invoked with (), and delete is neither a function nor an object. You will have to wrap it up in a function (or function object), perhaps like:

struct Deleter
{
    void operator()(Obj* obj) {delete obj;}
};

std::for_each(objs.begin(), objs.end(), Deleter());

But you should be very careful managing object lifetimes with raw pointers, especially if you're passing them around. You'll need to remember to delete them if you erase them from the vector, or reassign them, or if you clear the vector, or if an exception, break or function return might cause the vector to be destroyed. In general, it's always better to separate the responsibilities of resource management and resource usage.

You'd be better off with a vector of objects, unless Obj is a polymorphic base class, or the objects really are big or complicated enough that copying them will have a noticeable impact on performance. If that is the case (and you've profiled it to be sure that it's the case), you should consider a vector of smart pointers (shared_ptr, or unique_ptr if your compiler supports it), or Boost's ptr_vector.

Getting in the habit of using automatic resource management classes will save you a lot of headaches in the future.

Mike Seymour
I think `for_each` only appears to take a function, because that function first decays to a function pointer. And that function pointer is an object which can be invoked with `()`.
MSalters
@MSalters: Yes, that's right.
Mike Seymour
+1  A: 

Instead of trying to solve the deletion problem, you can make it go away completely by storing shared_ptrs in the vector, or by using boost's ptr_vector (see http://www.boost.org/doc/libs/1_39_0/libs/ptr_container/doc/tutorial.html).

Mark B
A: 

Yes. Fill it with smart pointers and use vector.clear() is the easiest way.

DeadMG