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

[lallous]

#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

 

[Jay]

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.

 

[Juha Nieminen]

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

 

[Ramon F Herrera]

#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]

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

 

[lallous]

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