std::remove_if: Removing all occurences of a pointer from std::vector

Question

I have an std::vector instance as defined in the following line:

std::vector< std::pair<EndPointAddr*, EndPointAddr*>* > mServiceSubscriptionsList;

The first item in the underlying std::pair object is the network address of the subscriber entity while the second is the network address of the subscribed entity. So, an std::pair object represents a subscription here as pair of subscriber and subscribed endpoint addresses.

I would like to delete all subscriptions in this vector for a given subscriber endpoint address. For this purpose, I wrote below indicated function where I use std::remove_if with a predicate. Based on the documentation of std::remove_if, my understanding is that std::remove_if puts all occurrences to be deleted to the end of the vector and moves the end of the vector backward to its new position.

My questions is:

How can I reach to these std::pair items that are put into the end of the vector after the call to remove_if in order to deallocate their contents dynamically one by one (i.e. dellocating std::pair* pointers)? Could you indicate the needed code snippet in the below function code? I can delete the first occurence kept in the iterator last. However, I am not sure how can I delete the rest of the occurences. Thanks.

bool 
XXX::removeSubscriptionForASpecificSubscriber(EndPointAddr * ptrSubscriberAddr)
{
  auto last = 
       std::remove_if(mServiceSubscriptionsList.begin(),
                      mServiceSubscriptionsList.end(),
                      [ptrSubscriberAddr](std::pair<EndPointAddr*, EndPointAddr*>*  thePair) 
                      { 
                         return ptrSubscriberAddr->getXXXAddress().compareTo(thePair->first->getXXXAddress());
                      });

 if(last != mServiceSubscriptionsList.end())
 {

   //HERE I CAN DELET THE FIRST OCCURENCE, but WHAT I WANT IS TO DELETE ALL OCCURANCES
   if(*last != nullptr)
   { 
     delete *last;
   }

   mServiceSubscriptionsList.erase(last, mServiceSubscriptionsList.end());

   return true;
 }

 return false;
}

I think I better not allocate the std::pair dynamically and instead assign it as a non-pointer variable into the vector. This will bring less impact to the existing code. Thanks to all for the answers... — F. Aydemir
– F. Aydemir, Commented Jun 14, 2013 at 13:55

Luc Touraille · Accepted Answer · 2013-06-14 13:34:26Z

6

There is no guarantee that remove_if puts erased elements at the end of the vector: iterators in range [newEnd, oldEnd) are dereferencable, but the elements have unspecified value.

For instance, the following code

std::vector<int> v { 0, 1, 2, 3, 4 };
auto new_end = std::remove_if(v.begin(), v.end(), is_odd);

can modify v such that it contains

0, 2, 4, 3, 4
         ^
       newEnd

You should probably use std::partition instead, or store smart pointers so that you can use the erase-remove idiom (or even do not store pointers at all).

edited Jun 14, 2013 at 13:34

answered Jun 14, 2013 at 13:28

Luc Touraille

82.6k17 gold badges100 silver badges140 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

David Over a year ago

Why do those elements have an unspecified value?!

Luc Touraille Over a year ago

@Dave: Because the goal of remove_if is to remove elements, not to separate them from other elements in the range. Therefore, the algorithm makes the assumption that the erased elements do not need to be accessed afterwards.

Balog Pal · Accepted Answer · 2013-06-14 13:27:03Z

2

What is that delete supposed to do? at last..end contains 'obsolete' element trash for which the content was copied to the vector before it. Certainly you could call a for_each on the range with delete in ht lambda, but I doubt that would get sensible result.

If you want remove entries and also delete their content you need completely different approach. Like making the raw pointer unique_ptr instead.

answered Jun 14, 2013 at 13:27

Balog Pal

17.4k2 gold badges25 silver badges37 bronze badges

Comments

ChronoTrigger · Accepted Answer · 2013-06-14 13:29:35Z

If I understood correctly the documentation ("Removing is done by shifting the elements in the range in such a way that elements to be erased are overwritten"), the elements you need to delete are overwritten, so you cannot delete its dynamic content because you lose the pointers to the elements to be erased.

You should find first the indices in your vector of the elements to remove, deallocate them, and do the removal later. I suggest a solution similar to this: 1) use std::find_if to find the first element to remove, 2) deallocate the content and swap pointer with the "last" element of your vector, 3) repeat until std::find_if returns nothing. Here, "last" means the last element that has not been still flagged to be removed.

David · Accepted Answer · 2013-06-14 13:37:04Z

2

I'll offer 2 alternatives instead of showing how to delete your elements properly...

Best Solution: Don't dynamically allocate the pair:

std::vector<std::pair<EndPointAddr*, EndPointAddr*>>

Pretty simple. A pair which contains 2 pointers is tiny. It's going to be faster, and easier, to not dynamically allocate that pair. You don't need to worry about deletion either.

Acceptable Solution: Use unique_ptr:

If you know why you are dynamically allocating, and know that you have to in this case, use a smart pointer (unique_ptr). unique_ptr will clean itself up, so you don't need to delete anything.

std::vector<std::unique_ptr<std::pair<EndPointAddr*, EndPointAddr*>>>

answered Jun 14, 2013 at 13:37

David

28.3k18 gold badges95 silver badges141 bronze badges

Comments

Yakk - Adam Nevraumont · Accepted Answer · 2013-06-14 14:51:07Z

First, write erase_remove_if:

template<typename Container, typename Lambda>
Container&& erase_remove_if( Container&& c, Lambda&& closure ) {
  using std::begin; using std::end;
  auto new_end = std::remove_if( begin(c), end(c), std::forward<Lambda>(closure) );
  c.erase(new_end, end(c));
  return std::forward<Container>(c);
}

second, erase the data in your remove_if predicate:

bool removeSubscriptionForASpecificSubscriber(EndPointAddr * ptrSubscriberAddr)
{
  erase_remove_if( mServiceSubscriptionsList, 
    [ptrSubscriberAddr](std::pair<EndPointAddr*, EndPointAddr*>*  thePair) 
    {
      if (ptrSubscriberAddr->getXXXAddress().compareTo(thePair->first->getXXXAddress()))
      {
        delete ptrSubscriberAddr;
        return true;
      } else {
        return false;
      }
    });
  return true;
}

if you don't want to use std::unique_ptr to store your pairs-of-pointers. Note that if you have a std::vector which denotes ownership of the pointers within, it really is a nearly painless drop-in fix to make it a vector<unique_ptr<>>. You do have to delete some code that manages the memory, replace some push_back with emplace_back, and add some .get() calls, and that it.

Collectives™ on Stack Overflow

std::remove_if: Removing all occurences of a pointer from std::vector

5 Answers 5

2 Comments

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

2 Comments

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related