Cleaning up containers of pointers

[arnon1]

Hey. I'm relatively new to the forum.
Anyway, I've been trying to cleaning up the containers of pointers when they have same pointer into it. What changing need class Shape, Circle or/and template function purge for main can run without any exceptions. Here is program I try to run: (I work with Microsoft Visual C++ 2008.)

// Shape.cpp
#include <iostream>
#include <list>
#include "purge.h"
using namespace std;
class Shape {
public:
virtual void draw() = 0;
virtual ~Shape() {};
};

class Circle : public Shape {
public:
Circle(){}
Circle( const Circle& ){}
Circle& operator=( const Circle& ){ return *this; }
void draw() { cout << "Circle::draw" << endl; }
~Circle() { cout << "~Circle" << endl; }
};
int main() {
list<Shape*> shapes;
Circle* c1 = new Circle;
shapes.push_back(c1); //first time.
shapes.push_back( new Circle );
shapes.push_back( c1 ); //secned time.

purge(shapes);
}
//: :purge.h
#ifndef PURGE_H
#define PURGE_H

//class Shape;
template<class Seq>
void purge(Seq& c) {
typename Seq::iterator i;
for(i = c.begin(); i != c.end(); ++i) {
delete *i;
*i = 0;
}
}
#endif
Thank you for your time.

[jsmith]

Why not use

std::list<boost::shared_ptr<Shape>>

?

[GPN]

You can use c.sort(); c.unique(); in the template before the for loop. The list is sorted first and the repeated pointers are removed.

[kbw]

GPN, I think you've missed the point. The question is about pointer ownership. They can't all call delete on the pointers and the ownership isn't defined as it stands.

[Robertlzw]

I do not have time checking your code thoroughly, but you would double check the function template purge(Seq& c) you used. The c you declared is a reference to a type Seq (template parameter). Since it is not a container, how can you use c.begin(), etc. ?

The purge() function template should at least look like this:

template<class Seq>
void purge(list<Seq*>& c) {... }

rather than:

template<class Seq>
void purge(Seq& c) {...}

Furthermore, the for(i = c.begin(); i != c.end(); ++i) { ...} is not the right pattern to cleaning up a containers of pointers: you need not only delete the object that each pointer element pointed to through *iterator, as you did, which is correct, but also remove the pointer element itself (ptr1 up to the end) from the container via current iterator given a list as (prt1, ptr2, ptr2, ...). Try something as follows instead:

template<class Seq>
void purge(list<Seq*>& c) 
{
typename list<Seq*>::iterator i;
i = c.begin();
while(i != c.end() ) 
{
delete *i;   
*i = 0; // not necessary, no harm though
delete i; // remove the current pointer element from the container: EDIT this line, plz
i = c.begin();// always delete from the beginning of the container in the next loop
}
}

I hope this helps.

EDIT: line 10 should be read as: c.erase(i);
Sorry for finding it out many days later.