CODEWITHSUNDEEP
c++stdvector
F. Aydemir
F. Aydemir

Reputation: 2735

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

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;
}

Upvotes: 3

Views: 1949

Answers (5)

Yakk - Adam Nevraumont
Yakk - Adam Nevraumont

Reputation: 275976

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.

Upvotes: 0

David
David

Reputation: 28178

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*>>>

Upvotes: 2

Luc Touraille
Luc Touraille

Reputation: 82171

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).

Upvotes: 6

ChronoTrigger
ChronoTrigger

Reputation: 8617

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.

Upvotes: 2

Balog Pal
Balog Pal

Reputation: 17243

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.

Upvotes: 2

Related Questions