Thread: Accidentally changing class members??

I've been having the strangest bug in my program. Maybe I'm overlooking something obvious but here's what I'm doing. I have a polygon class which dynamically allocates memory for an array of points:

Code:

typedef struct _PCOORD
{
float x,y;
} PCOORD, *PTRPCOORD;
//..in polygon class
PCOORD* vertex;

//...and here is how I allocate space for the array
void _POLYGON2D::Resize(int size)
{
delete vertex;
vertex = new PCOORD[size];
vertices = size;
}

I call the Resize function once before I set the X and Y coordinates. Later I use a line drawing function to draw the polygon, by simply looping through all the points:

Code:

for (int i=0;i<poly.Size();i++)
 Draw_Line(poly.X(i),poly.Y(i),poly.X(i+1),poly.Y(i+1),0,(UINT*)ddsd.lpSurface,int(ddsd.lPitch>>2));

the X() and Y() functions are defined like this:

Code:

float _POLYGON2D::X(int index)
{
if (vertices==0)
 return 0;
if (index<0)
 return vertex[vertices-1].x;
else if (index<vertices)
return (vertex[index].x+center.x);
else if (index>=vertices)
return (vertex[0].x+center.x);


}

float _POLYGON2D::Y(int index)
{
if (vertices==0)
 return 0;
if (index<0)
 return vertex[vertices].y;
else if (vertices==0)
 return 0;
else if (index<vertices)
return (vertex[index].y+center.y);
else if (index>=vertices)
 return (vertex[0].y+center.y);
}

This way I can use that loop and when it goes above the number of vertices it will draw back to the first one...Now this seems to work fine until I get to another function I'm working on. I'm trying to make it a filled polygon, and although my algorithm may not work, it should not even touch the members of my class:

Code:

inline int Fill_Poly(Polygon2D poly,UINT* vb_start, int lpitch32)
{
float* SX,*SY;
SX=new float[poly.Size()];
SY=new float[poly.Size()];

for (int i=0;i<poly.Size();i++)
 {
 SX[i]=poly.X(i);
 SY[i]=poly.Y(i);
 
 }
 delete SX;
  delete SY;
}

Note there is a bunch of code in the middle which is commented out, but I have narrowed it down to those two lines
Somehow, when those two lines are in there, the vertices change about every second and sort of randomly move down and right...I have no idea how calling those functions could change the values in the PCOORD array

1) You need delete[] not delete for arrays.

2) What is Polygon2D's copy constructor like? Is it shallow or deep?

3) Take the time to make your code const-correct. If a method doesn't change the state of the object, tell the compiler. If a function won't alter an object that it receives by pointer or reference, tell the compiler that, too. If you don't want the user to modify an object you return -- the compiler should know that as well.

const-correctness is a great idea, and you don't follow it at all.

Thanks, Cat. It all makes sense now...Of course I had competely omitted a copy constructor (it hadn't occurred to me that I would need one if I didn't explicitly call it)...I'll definitely remember this in the future

As for "const-correctness" you mean like making the functions and arguments const?

Code:

float X(const int index) const;
float Y(const int index) const;

I'll pay more attention to that in the future as well, although I can't guarantee I will go about changing my classes for this particular project (which I was trying to keep simple). I'm beginning to see that designing good classes makes for much less error-prone code

Originally posted by JaWiB
Thanks, Cat. It all makes sense now...Of course I had competely omitted a copy constructor (it hadn't occurred to me that I would need one if I didn't explicitly call it)...I'll definitely remember this in the future

As for "const-correctness" you mean like making the functions and arguments const?

Code:

float X(const int index) const;
float Y(const int index) const;

I'll pay more attention to that in the future as well, although I can't guarantee I will go about changing my classes for this particular project (which I was trying to keep simple). I'm beginning to see that designing good classes makes for much less error-prone code

Even simple projects should be const-correct.

And you DO need a copy constructor, because your Fill_Poly method is taking a *copy* of a Polygon2D object. So what probably happens is your memory is being deallocated when that temporary is destroyed.

Remember, it auto-generates a copy constructor if you don't write one.