mjhjmtu

I have following pattern:

1. I have a std::vector containing raw pointers to objects (I know raw pointers are "evil", but it's legacy software needing to be maintained).


2. Now for each element in the vector I need to do a test and if the test is positive do something with the pointer, delete it and then remove it from the vector:

Pseudo code:

for each pointer in vector

 if (SomeTest(pointer))
 
 DoSomething(pointer)
 delete pointer
 remove pointer from vector
 

I'm unable to come up with some nice clean code for this.

This link provides different approaches, but they all look more or less cumbersome to me.

Cumbersome solution I'm using now:

for(auto & p : v)

 if (SomeTest(p))
 
 DoSomething(p);
 delete p;
 p = nullptr;
 


v.erase(std::remove(v.begin(), v.end(), nullptr), v.end());

As often the answer is: know your <algorithm>s (and is a good reminder to myself) ;)

std::partition is what you're looking for: std::partition(begin, end, p) "moves" the elements of the range [begin, end) which do not satisfy the predicate p at the end of the range; you can then treat them as a batch.

auto const to_be_removed = std::partition(begin(v), end(v), (auto p) /* predicate */ );
std::for_each(to_be_removed, end(v), (auto p) 
 /* crunch */
 delete p;
);
v.erase(to_be_removed, end(v));

Full program

#include <iostream>
#include <algorithm>
#include <vector>

int main()

 std::vector v = new int0, new int1, new int2 ;

 // let's delete all even values
 auto const to_be_removed = std::partition(begin(v), end(v), (auto p) return *p % 2 != 0; );
 std::for_each(to_be_removed, end(v), (auto p) 
 std::cout << "Deleting value " << *p << "...n";
 delete p;
 );
 v.erase(to_be_removed, end(v));

Live demo

To go further

This implementation has two major drawbacks: the order from the vector is not stable (1), it could be factored into a reusable function (2).

• (1) is solved by std::stable_partition.


• (2) is not that hard:

template<class InputIt, class UnaryPredicate, class UnaryDeleter>
InputIt delete_if(InputIt begin, InputIt end, UnaryPredicate p, UnaryDeleter d)

 auto const to_be_removed = std::stable_partition(begin, end, std::not_fn(p));
 std::for_each(to_be_removed, end, [d](auto p) d(p) ; delete p; );
 return to_be_removed;


template<class Container, class UnaryPredicate, class UnaryDeleter>
auto delete_if(Container& c, UnaryPredicate p, UnaryDeleter d)

 using std::begin, std::end;
 return c.erase(delete_if(begin(c), end(c), p, d), end(c));

Usage:

delete_if(v, SomeTest, DoSomething);

Live demo

You can use std::remove_if I am not sure why the article you linked uses std::remove_if before deleting the pointers because that won't work. You have to delete the pointers before the removal:

std::vector<int*> v;

v.erase(std::remove_if(std::begin(v), std::end(v), (int* p)

 // do your test and do not remove on failure
 if(!SomeTest(p))
 return false; // keep this one

 DoSomething(p);

 // Now we remove but be sure to delete here, before the
 // element is moved (and therefore invalidated)

 delete p;

 return true; // signal for removal

), std::end(v));

Notes: Why this is safe.

Deleting the pointer does not modify the pointer itself, but the object being pointed to. That means that no elements are modified with this approach.

The standard at C++17 28.6.8 5 guarantees that the predicate will be called just once for each element.

The simplest solution - starting from the linked article - is to take the erase_if function

template <typename Container, typename Pred>
void erase_if(Container &c, Pred p)

 c.erase(std::remove_if(std::begin(c), std::end(c), p), std::end(c));

and just call it with

erase_if(v, (T *pointer)
 
 if (SomeTest(pointer))
 
 DoSomething(pointer);
 delete pointer;
 return true; //remove pointer from vector
 
 return false;
 );

You can obviously split your predicate in two if you want to separate the SomeTest/DoSomething part from the delete part:

template <typename Container, typename Pred>
void delete_if(Container &c, Pred p)

 auto e = std::remove_if(std::begin(c), std::end(c),
 [&p](Container::value_type *pointer)
 
 if (p(pointer)) 
 delete pointer;
 return true;
 
 return false;
 );
 c.erase(e, std::end(c));

Since you haven't said why you don't like the erase_if you linked yourself, I can't guess whether this has the same problem.