Problem with destruction using iterators

[navderm]

I have a vector which saves a face of a triangulation which has three points in 3D. I create each face using new (face is a class). When i try to use destructor and write the following code, apparently cntr cannot be used in delete.

How do i delete all the elements stored in the vector

vector<Face>::iterator cntr;
for (cntr = allFaces.begin(); cntr < allFaces.end(); cntr++)
    delete cntr;

Thx !
navderm

[hanst99]

iterators may look like pointers, but they aren't really. Is there any reason why you would want to work with pointers instead of with normal objects (How about I answer that for you - if you don't need polymorphy the answer is no)?

Well, anyways, to delete pointers you put into a vector you would have to do this:
delete *cntr;

[Disch]

You have a few problems:

1) Don't delete iterators. You delete pointers.

2) If you have vector<Face> then your vectors contain Face objects, not pointers. Therefore there's nothing for you to delete.

3) If you're creating something with new, you probably have memory leaks. Probably what's happening is you're creating a new Face, then copying that new Face to another Face in the vector, and never deleting the new'd Face.


So you have a few options options here.

Option 1) keep vector<Face> and don't use new:

vector<Face> vec;

vec.push_back( Face(whatever) ); // add another Face to the vector
  // notice:  not using new

// no need to do any cleanup 

Option 2) Keep new, and change it to a vector<Face*> (ie: the vector has pointers)

vector<Face*> vec;

vec.push_back( new Face(whatever) );

// then to clean up:
while(!vec.empty())
{
  delete vec.back();
  vec.pop_back();
}

Option 3) keep new and use a container like boost::ptr_vector and have it manage the memory so you don't have to clean up:

boost::ptr_vector<Face> vec;

vec.push_back( new Face(whatever) );

// no need to clean up, boost::ptr_vector will delete the stuff for you 

[navderm]

Hi.
Actually this is a function which takes in points, triangulates them, makes faces and then copies it into the vector which is a private member of mesh.
Hence to copy in vector using push_back (which is a reference copy) i need to create a new object of face and then copy it into the vector. Thats the reason I was using "new".
Yes, though i create a pointer , i copy the value , which, according to my meagre knowledge, should work because its just copying the reference.

int Mesh::triangulatePtsAddFace(int pointCnt, vector<DsPoint> allPoints)
{
	vector<DsPoint>::iterator cntr;
	int faceCount = 0;
	for (cntr = allPoints.begin(); cntr < allPoints.end() - 2; cntr++)
	{
		Face *newFace;
		newFace = new Face(*allPoints.begin(), *(cntr+1), *(cntr+2));
		this->addFace(*newFace);
		faceCount++;
	}
	return faceCount;
}

So, I try to delete all the objects created using "new" in the destructor of Mesh.

Is there any other way anyone can think of , of creating these faces other than new in this situation ? I don't want to end up returning values from the function. I already have the no. of faces being returned.

@Disch
The first option seems right. But when i create that Face in a function, does it not get deleted? push_back being reference copy will again give me error. Kindly correct me if I am wrong.
I'll try looking into boost libraries.

Thanks a ton!
navderm

[hanst99]

Ouch. Looks like you got some conceptual misconceptions.

int Mesh::triangulatePtsAddFace(int pointCnt, vector<DsPoint> allPoints)
{
	vector<DsPoint>::iterator cntr;
	int faceCount = 0;
	for (cntr = allPoints.begin(); cntr < allPoints.end() - 2; cntr++)
	{
                //you are just leaking memory here - a lot of it too.
		Face *newFace;
		newFace = new Face(*allPoints.begin(), *(cntr+1), *(cntr+2));
		this->addFace(*newFace);
                //Does the same thing, no new needed, no memory leaks created
                this->addFace(Face(*allPoints.begin(),*(cntr+1),*(cntr+2));
		faceCount++;
	}
	return faceCount;
}

push_back creates a copy of the value it is being passed, so pushing back a temporary value is completely ok.

[navderm]

Oh! That was a huge misconception ! Thanx a lot !
ya! that was indeed a lot of memory leak !

navderm