Deleting items from an std::list , is this code correct?

Discussion in 'C++' started by lallous, Apr 23, 2008.

  1. lallous

    lallous Guest

    #include <conio>
    #include <list>

    typedef std::list<int> int_list_t;
    typedef std::list<int_list_t::iterator> int_list_iterator_list_t;

    void print_list(int_list_t &L)
    {
    for (int_list_t::iterator it=L.begin();it!=L.end();++it)
    {
    std::cout << "value = " << *it << std::endl;
    }
    }

    void delete_odd(int_list_t &L)
    {
    int_list_iterator_list_t it_list;
    int_list_t::iterator it;

    for (it=L.begin();it!=L.end();++it)
    {
    if (*it % 2 != 0)
    it_list.push_back(it);
    }

    for (int_list_iterator_list_t::const_iterator di=it_list.begin();di!
    =it_list.end();++di)
    {
    L.erase(*di);
    }
    }

    void populate_list(int_list_t &L, int start, int end)
    {
    L.clear();
    for (int i=start;i<=end;i++)
    L.push_back(i);
    }

    int main()
    {
    int_list_t L;

    populate_list(L, 1, 10);
    print_list(L);

    std::cout << "---------------------" << std::endl;

    delete_odd(L);
    print_list(L);

    return 0;
    }

    Please advise. Does this work with all STL implementations?

    Thank you,
    Elias
     
    lallous, Apr 23, 2008
    #1
    1. Advertising

  2. lallous

    Jay Guest

    There's one thing you want to watch out for when using containers.
    With some of them you can delete items and continue using the iterator
    and with some you can not.
    This seems fine, but doesn't work with all containers:

    for ( int i = 0; i < 10; ++i )
    L.push_back( i );
    std::vector<int>::iterator it;
      for ( it=L.begin(); it!=L.end(); ++it )
    if( *it == 5 )
    L.erase( it );

    blows up because the iterator is invalidated after the delete.
     
    Jay, Apr 23, 2008
    #2
    1. Advertising

  3. lallous wrote:
    > void delete_odd(int_list_t &L)
    > {
    > int_list_iterator_list_t it_list;
    > int_list_t::iterator it;
    >
    > for (it=L.begin();it!=L.end();++it)
    > {
    > if (*it % 2 != 0)
    > it_list.push_back(it);
    > }
    >
    > for (int_list_iterator_list_t::const_iterator di=it_list.begin();di!
    > =it_list.end();++di)
    > {
    > L.erase(*di);
    > }
    > }


    That's a rather contrived (and inefficient) way of doing it. Why not
    simply:

    void delete_odd(int_list_t& L)
    {
    for(int_list_t::iterator iter = L.begin(); iter != L.end();)
    if(*it % 2 != )
    iter = L.erase(iter);
    else
    ++iter;
    }
     
    Juha Nieminen, Apr 23, 2008
    #3
  4. On Apr 23, 3:21 pm, lallous <> wrote:
    > #include <conio>
    > #include <list>
    >
    > typedef std::list<int> int_list_t;
    > typedef std::list<int_list_t::iterator> int_list_iterator_list_t;
    >
    > void print_list(int_list_t &L)
    > {
    > for (int_list_t::iterator it=L.begin();it!=L.end();++it)
    > {
    > std::cout << "value = " << *it << std::endl;
    > }
    >
    > }
    >
    > void delete_odd(int_list_t &L)
    > {
    > int_list_iterator_list_t it_list;
    > int_list_t::iterator it;
    >
    > for (it=L.begin();it!=L.end();++it)
    > {
    > if (*it % 2 != 0)
    > it_list.push_back(it);
    > }
    >
    > for (int_list_iterator_list_t::const_iterator di=it_list.begin();di!
    > =it_list.end();++di)
    > {
    > L.erase(*di);
    > }
    >
    > }
    >
    > void populate_list(int_list_t &L, int start, int end)
    > {
    > L.clear();
    > for (int i=start;i<=end;i++)
    > L.push_back(i);
    >
    > }
    >
    > int main()
    > {
    > int_list_t L;
    >
    > populate_list(L, 1, 10);
    > print_list(L);
    >
    > std::cout << "---------------------" << std::endl;
    >
    > delete_odd(L);
    > print_list(L);
    >
    > return 0;
    >
    > }
    >
    > Please advise. Does this work with all STL implementations?
    >
    > Thank you,
    > Elias



    After making a minor modification, your code runs properly under MSVS
    ang gcc.

    I changed the line:

    #include <conio>
    to
    #include <iostream>

    -RFH
     
    Ramon F Herrera, Apr 23, 2008
    #4
  5. lallous

    lallous Guest

    Ramon: my bad, I mistyped iostream for conio
    Victor: The idea of the code is to ask if it is possible to store
    iterators and later use them.
    Juha: looks nice, I didn't notice that erase returns another iterator
    beyond the one that is just deleted!

    I haven't tried it, but theoretically, this shouldn't work, no?

    void delete_odd(int_list_t &L)
    {
    int_list_iterator_list_t it_list;
    for (int_list_t::iterator it=L.begin();it!=L.end();++it)
    {
    if (*it % 2 != 0)
    it_list.push_back(it);
    }

    for (int_list_iterator_list_t::const_iterator
    di=it_list.begin();di!
    =it_list.end();++di)
    {
    L.erase(*di);
    }
    }

    I only moved the declaration of "it" to inside the loop, that should
    free the iterator by the end of the loop, thus rendering all the
    stored iterators in it_list unusable?

    Regards,
    Elias
    On Apr 24, 1:18 am, Ramon F Herrera <> wrote:
    > On Apr 23, 3:21 pm,lallous<> wrote:
    >
    >
    >
    > > #include <conio>
    > > #include <list>

    >
    > > typedef std::list<int> int_list_t;
    > > typedef std::list<int_list_t::iterator> int_list_iterator_list_t;

    >
    > > void print_list(int_list_t &L)
    > > {
    > > for (int_list_t::iterator it=L.begin();it!=L.end();++it)
    > > {
    > > std::cout << "value = " << *it << std::endl;
    > > }

    >
    > > }

    >
    > > void delete_odd(int_list_t &L)
    > > {
    > > int_list_iterator_list_t it_list;
    > > int_list_t::iterator it;

    >
    > > for (it=L.begin();it!=L.end();++it)
    > > {
    > > if (*it % 2 != 0)
    > > it_list.push_back(it);
    > > }

    >
    > > for (int_list_iterator_list_t::const_iterator di=it_list.begin();di!
    > > =it_list.end();++di)
    > > {
    > > L.erase(*di);
    > > }

    >
    > > }

    >
    > > void populate_list(int_list_t &L, int start, int end)
    > > {
    > > L.clear();
    > > for (int i=start;i<=end;i++)
    > > L.push_back(i);

    >
    > > }

    >
    > > int main()
    > > {
    > > int_list_t L;

    >
    > > populate_list(L, 1, 10);
    > > print_list(L);

    >
    > > std::cout << "---------------------" << std::endl;

    >
    > > delete_odd(L);
    > > print_list(L);

    >
    > > return 0;

    >
    > > }

    >
    > > Please advise. Does this work with all STL implementations?

    >
    > > Thank you,
    > > Elias

    >
    > After making a minor modification, your code runs properly under MSVS
    > ang gcc.
    >
    > I changed the line:
    >
    > #include <conio>
    > to
    > #include <iostream>
    >
    > -RFH
     
    lallous, Apr 24, 2008
    #5
  6. lallous

    lallous Guest

    Hi Juha,

    I can think of a case where there are two functions, one that
    processes items and marks them for deletion (by adding their "it" to a
    list) and another that processes them delete list and erases the
    items.

    You have other suggestions for such case?

    Thanks,
    Elias
    On Apr 24, 12:54 am, Juha Nieminen <> wrote:
    > lallouswrote:
    > > void delete_odd(int_list_t &L)
    > > {
    > > int_list_iterator_list_t it_list;
    > > int_list_t::iterator it;

    >
    > > for (it=L.begin();it!=L.end();++it)
    > > {
    > > if (*it % 2 != 0)
    > > it_list.push_back(it);
    > > }

    >
    > > for (int_list_iterator_list_t::const_iterator di=it_list.begin();di!
    > > =it_list.end();++di)
    > > {
    > > L.erase(*di);
    > > }
    > > }

    >
    > That's a rather contrived (and inefficient) way of doing it. Why not
    > simply:
    >
    > void delete_odd(int_list_t& L)
    > {
    > for(int_list_t::iterator iter = L.begin(); iter != L.end();)
    > if(*it % 2 != )
    > iter = L.erase(iter);
    > else
    > ++iter;
    >
    > }
     
    lallous, Apr 24, 2008
    #6
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. NFish
    Replies:
    6
    Views:
    15,290
    Andrey Tarasevich
    Nov 7, 2003
  2. Replies:
    6
    Views:
    653
    Jim Langston
    Oct 30, 2005
  3. Markus Svilans

    Deleting items from std::list

    Markus Svilans, Jun 26, 2006, in forum: C++
    Replies:
    25
    Views:
    1,437
    Robbie Hatley
    Jun 28, 2006
  4. Andy
    Replies:
    3
    Views:
    488
    James Kanze
    Jun 8, 2007
  5. Juha Nieminen
    Replies:
    22
    Views:
    1,036
    Kai-Uwe Bux
    Oct 12, 2007
Loading...

Share This Page