deleting list objects

D

dasod

I would like to know if my method to remove list objects is correct in
this small test program. It seems to me that there might be a simplier
way, but I'm afraid I don't know enough about list iterators and how
they are behaving in situations like this.

#include <iostream>
#include <list>

class Test;
typedef std::list< Test* > Tlist;
Tlist objects;

class Test
{
private:
bool Active;
int xx;
public:
Test(bool a, int x)
: Active(a), xx(x) {}
void print() { std::cout << Active << " \t" << xx << std::endl; }
bool is_active() { return Active; }
};

void print_all (const char *msg)
{
std::cout << msg << std::endl;
for (Tlist::iterator i = objects.begin(); i != objects.end(); ++i)
(*i)->print();
}
void remove_active_objects()
{
for (Tlist::iterator i = objects.begin(); i != objects.end(); )
{
if ((*i)->is_active())
{
Tlist::iterator temp = i;
++i;
objects.erase(temp);
delete *temp;
}
else
{
++i;
}
}
}

int main()
{
objects.push_back (new Test(true, 12));
objects.push_back (new Test(false, 345));
objects.push_back (new Test(false, 6789));
objects.push_back (new Test(true, 19283));

print_all ("Before:");
remove_active_objects();
print_all ("After:");

for (Tlist::iterator i = objects.begin(); i != objects.end(); ++i)
delete *i;

return 0;
}

---------------------------------
Output of this program should be:

Before:
1 12
0 345
0 6789
1 19283
After:
0 345
0 6789
 
J

John Harrison

dasod said:
I would like to know if my method to remove list objects is correct in
this small test program. It seems to me that there might be a simplier
way, but I'm afraid I don't know enough about list iterators and how
they are behaving in situations like this.

#include <iostream>
#include <list>

class Test;
typedef std::list< Test* > Tlist;
Tlist objects;

class Test
{
private:
bool Active;
int xx;
public:
Test(bool a, int x)
: Active(a), xx(x) {}
void print() { std::cout << Active << " \t" << xx << std::endl; }
bool is_active() { return Active; }
};

void print_all (const char *msg)
{
std::cout << msg << std::endl;
for (Tlist::iterator i = objects.begin(); i != objects.end(); ++i)
(*i)->print();
}
void remove_active_objects()
{
for (Tlist::iterator i = objects.begin(); i != objects.end(); )
{
if ((*i)->is_active())
{
Tlist::iterator temp = i;
++i;
objects.erase(temp);
delete *temp;

This is not correct, the iterator temp (which is just a copy of i) is no
longer valid after you've erased the item.

if ((*i)->is_active())
{
delete *i;
i = objects.erase(i);
}
else
{
++i;
}

john
 
D

dasod

John Harrison said:
This is not correct, the iterator temp (which is just a copy of i) is no
longer valid after you've erased the item.

if ((*i)->is_active())
{
delete *i;
i = objects.erase(i);
}
else
{
++i;
}

Yes, it looks better now. Thanks a lot.
 

Ask a Question

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

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top