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

  • Follow


#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
0
Reply lallous (276) 4/23/2008 7:21:57 PM

lallous wrote:
> [..]
>
> Please advise. Does this work with all STL implementations?

Seems OK.  But why wouldn't you simply remove all odd elements
using the 'remove_if' function?

V
-- 
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask 


0
Reply v.Abazarov (13255) 4/23/2008 7:34:53 PM


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 =3D 0; i < 10; ++i )
      L.push_back( i );
   std::vector<int>::iterator it;
 =A0 for ( it=3DL.begin(); it!=3DL.end(); ++it )
     if( *it =3D=3D 5 )
       L.erase( it );

blows up because the iterator is invalidated after the delete.


0
Reply JSprenkle (4) 4/23/2008 7:55:57 PM

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;
}
0
Reply nospam270 (2853) 4/23/2008 9:54:10 PM

On Apr 23, 3:21 pm, lallous <lall...@lgwm.org> 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

0
Reply ramon (1465) 4/23/2008 10:18:30 PM

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 <ra...@conexus.net> wrote:
> On Apr 23, 3:21 pm,lallous<lall...@lgwm.org> 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

0
Reply lallous (276) 4/24/2008 5:26:44 AM

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 <nos...@thanks.invalid> 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;
>
> }

0
Reply lallous (276) 4/24/2008 5:30:59 AM

6 Replies
25 Views

(page loaded in 0.122 seconds)


Reply: