5

Is it possible to access the std::for_each iterator, so I can erase the current element from an std::list using a lambda (as below)

typedef std::shared_ptr<IEvent>    EventPtr;
std::list<EventPtr> EventQueue;
EventType evt;
...

std::for_each( 
    EventQueue.begin(), EventQueue.end(),

    [&]( EventPtr pEvent )
    {
        if( pEvent->EventType() == evt.EventType() )
            EventQueue.erase( ???Iterator??? );
    }
);

I've read about using [](typename T::value_type x){ delete x; } here on SO, but VS2010 doesn't seem to like this statement (underlines T as error source).

James McNellis
  • 348,265
  • 75
  • 913
  • 977
fishfood
  • 4,092
  • 4
  • 28
  • 35

3 Answers3

8

You are using the wrong algorithm. Use remove_if:

EventQueue.remove_if([&](EventPtr const& pEvent)
{
    return pEvent->EventType() == evt.EventType();
});

The STL algorithms do not give you access to the iterator being used for iteration. This is in most cases a good thing.

(In addition, consider whether you really want to use std::list; it's unlikely that it is the right container for your use case. Consider std::vector, with which you would use the erase/remove idiom to remove elements that satisfy a particular predicate.)

James McNellis
  • 348,265
  • 75
  • 913
  • 977
  • I'd started out with an std::queue, but "upgraded" to a list because I do need to iterate the entire queue for some functions. Other than that, I thought a list would be more efficient than a vector that may grow/shrink over time. – fishfood Jun 28 '12 at 14:37
  • 1
    `std::list` is implemented as a doubly-linked list; it rarely has better performance characteristics than `std::vector`, which is implemented using an array. The time spent moving objects in array (especially small objects like `shared_ptr` objects) is far less than the time required to walk linked list nodes and dynamically allocate and destroy nodes. `std::vector` should be the default choice for a sequence container; only switch to another if performance is a problem and profiling shows that another container would perform better. – James McNellis Jun 28 '12 at 14:39
  • Isn't a vector bad for inserting data in the middle though? I thought efficient insert was the advantage of using linked lists. – fishfood Jun 28 '12 at 14:43
  • 1
    It depends, but usually not. Finding the location at which the data needs to be inserted is far more expensive for a `list` because the node links must be walked, one-by-one, until the right position is found. Every link is a pointer, which requires indirection to some arbitrary location in memory. In a `vector`, the elements are stored contiguously, which means it is much, much friendlier to modern CPU features (caches, prefetchers). There are some corner cases where a `list` _might_ perform better, but they are few and very,very far between. – James McNellis Jun 28 '12 at 18:13
1

no, use a regular for instead.

for( auto it = EventQueue.begin(); it != EventQueue.end(); ++it )
{
  auto pEvent = *it;
  if( pEvent->EventType() == evt.EventType() )
      it = EventQueue.erase( it );
);
smerlin
  • 6,446
  • 3
  • 35
  • 58
  • I'm kinda disappointed by that - it reduces the usefulness of lambdas, thanks though! – fishfood Jun 28 '12 at 14:28
  • 2
    @lapin: no, but you tried to use std::for_each for something which it is not intended for. If you want to remove something from a container, std::for_each is the wrong tool. Use erase + std::remove_if instead. And std::remove_if works with lambdas aswell of course... – smerlin Jun 28 '12 at 14:34
0

Erase is not the only time you may need to know iterator from lambda. To do this in a more general way, I am using & operator (implicit conversion to iterator) like this :

int main (int argc, char* argv []) {
  size_t tmp [6] = {0, 1, 2, 3, 4, 5};
  std::list<size_t> ls ((size_t*)tmp, (size_t*) &tmp [6]);
  //printing next element
  std::for_each ((const size_t*)tmp, (const size_t*) &tmp [5], [] (const size_t& s) {
    std::cout << s << "->";
    std::cout << *(&s +1) << "   ";
  });
  std::cout << std::endl;
}
Saint-Martin
  • 306
  • 3
  • 16