Cleaning up an STL list/vector of pointers

Question

What is the shortest chunk of C++ you can come up with to safely clean up a std::vector or std::list of pointers? (assuming you have to call delete on the pointers?)

list<Foo*> foo_list;

I'd rather not use Boost or wrap my pointers with smart pointers.

Solution 1

Since we are throwing down the gauntlet here... "Shortest chunk of C++"

static bool deleteAll( Foo * theElement ) { delete theElement; return true; }

foo_list . remove_if ( deleteAll );

I think we can trust the folks who came up with STL to have efficient algorithms. Why reinvent the wheel?

Solution 2

For std::list<T*> use:

while(!foo.empty()) delete foo.front(), foo.pop_front();

For std::vector<T*> use:

while(!bar.empty()) delete bar.back(), bar.pop_back();

Not sure why i took front instead of back for std::list above. I guess it's the feeling that it's faster. But actually both are constant time :). Anyway wrap it into a function and have fun:

template<typename Container>
void delete_them(Container& c) { while(!c.empty()) delete c.back(), c.pop_back(); }

Solution 3

for(list<Foo*>::const_iterator it = foo_list.begin(); it != foo_list.end(); ++it)
{
    delete *it;
} 
foo_list.clear();

Solution 4

If you allow C++11, you can do a very short version of Douglas Leeder's answer:

for(auto &it:foo_list) delete it; foo_list.clear();

Solution 5

It's really dangerous to rely on code outside of the container to delete your pointers. What happens when the container is destroyed due to a thrown exception, for example?

I know you said you don't like boost, but please consider the boost pointer containers.

View more solutions

Comments

[Mark Ransom]

Technically correct, but it gets a lot longer if you use more common brace and indenting conventions.

[Johannes Schaub - litb]

read it from left to right: While foo is not empty, delete the front of foo, and pop the front of foo :p newlines would only get into the way :/

[John Dibling]

The functor approach may not win for brevity, but that's not much of a prize. By using a functor you avoid having to write your own for loop, which is a source of many defects in software.

[CB Bailey]

The question specifically asked about "shortest". I had no idea that manual for loops were a major source of defects, can you provide a reference? If a member of my team had a problem writing a bug free for loop, I'd rather not let him loose on a functor solution.

[nnn]

I like it. I wrote a function template instead, but the remove_if idea is nice.

[kevinthompson]

I'd recommend against using the comma (sequence) operator. Too many C++ devs have no idea what it does, and will mistake it for a semicolon.

[Eternal Learner]

@Johannes Schaub - is it necessary to call delete. Isn't calling pop_back() sufficient? When I read what pop_back does , I see that it also calls the removed elements destructor. - cplusplus.com/reference/stl/vector/pop_back.

[a_m0d]

@EternalLearner Calling the destructor doesn't de-allocate memory. You still need to call delete on the pointer so that the memory is de-allocated.

[Raedwald]

One of the pitfalls is that, perversely, the STL allows several important iterator operations to throw exceptions. This makes many "obvious" approaches using iteration through a container unsafe. See stackoverflow.com/questions/7902452/…

[Raedwald]

The request was to "safely clean up" the container. But the safety of that code relies on several methods not throwing exceptions: begin(), end(), iterator::operator!=, iterator::operator*, iterator::operator++. Surpirsingly, such reliance is unsafe: stackoverflow.com/questions/7902452/…

[Andre Rodrigues]

Is it just me or does it feel really wrong when a Predicate have side effects?

[Dimitrios Menounos]

I like that! It kind of works like the shared_ptr but for the container only and it is so minimal and elegant. I was looking at the boost smart pointers the other day and it is over 200K - incomprehensible - source code!

[jsdw]

That's really neat! If you overload the pointer dereference (*) and member selection (->) operator too you can make it entirely transparent I would have thought.

[Christian Rau]

@YogeshArora Me too, but this doesn't make this a valid answer in any way.

[Christian Rau]

You know that delete foo already checks if foo is nullptr, do you?

[kendotwill]

@ChristianRau I do now. Thank You

[Adisak]

Upvoted because it performs well and is short. I added a modified version of your answer that is quite a bit shorter (but relies on C++11).

[peterh]

+1 because of the stability workaround. I think a big problem of the c++ is that this code more complex already as a such list manipulation in the plain old C.

[sp497]

The question did not specify const list. Why use const iterator ?

[Douglas Leeder]

@SRINI794 I used a const iterator to maximise for usability of the code, and because I wasn't going to alter the list during the iteration, so I didn't need a mutable iterator.

[Guy Avraham]

@DouglasLeeder Dont want to be petty, yet usually the increment of the iterator is done by: ++it and not it++. Anyhow, it does not affect the correctness of the solution.

[Matthew]

C++ 11 and C++ 14 allows you to use lamdas for arguably easier reading. the code becomes: foo_list.remove_if([](Foo *theElement){delete theElement; return true;});

[Matthew]

this is actually not a great idea for the simply reason that you are looping 2n times. once to loop through each element and deleting the memory then looping through the list again to clear the list and bring the list back down to zero. what you want to do is use an approach like Mr.Ree or Johannees Schaub's answer seen above. The both iterate over the list once performing both the deletion of memory and reducing of list size in one loop of size n.

[Sandburg]

Why a comma and not a semicolon? (for readability, confusion)

[Johannes Schaub - litb]

@Sandburg not sure.. I guess, it did fit without scrollbars. With semicolons, I need braces around. Why do you think semicolons would cause confusion?