segmentation fault when looping over 2D object array within constructor

[bobsta]

Hi,

I have been using C++ for a couple of months now (after many years programming
in FORTRAN) and have started my first
serious bit of programming (i.e. not following teach-yourself-book exercises).

I am reading in data from a file containing a plurality of geometric (anatomical)
structures (10 < i < 50) defined as a number of planes (1 < j < 50) where each
plane consists of a largish number of (x,y,z) points (10 < k < 500). To do this I
created two classes: a Point class for (x,y,z) points and a Structure class
for the geometric structures( which contains instances of the Point class).
In a first loop I read in the points and
determine the midpoint (x_0,y_0) for each contour but
now I want to extend my Point class methods to determine the
radius at each (x,y,z) point - as hypoteneuse from midpoint (x_0,y_0)_j for each
plane j i.e. I have to loop back over the points a second time to do
this. Below are the relevant parts of my code...

class Point{
      public:
        float x,y,z,r, x_0, y_0;
        Point();
        Point (float x, float y, float z, float r);
        ~Point();


        // determine radius at point (x,y)_j,k using mid-point (x_0, y_0)_k
        void setRadius(float x, float y, float x_0, float y_0);
	
	// return value
        float getRadius ();
};

Point::Point():x(0),y(0),z(0),r(0){
       x=y=z=r=0.0f;}

Point::Point(float x, float y, float z, float r):x(x),y(x),z(z),r(r){
                    r=0.0f;}

void Point::setRadius(float x, float y, float x_0, float y_0){
     	    float deltaX, deltaY;
	    deltaX = deltaY = 0.0f;
	    deltaX = x - x_0;
	    deltaY = y - y_0;
	    r = sqrt ((deltaX * deltaX) + (deltaY * deltaY));
}

float Point::getRadius(){return r;}

class Structure (...){
      Structure(); //parameterless constructor
      Structure(...); // constructor
...
};

Structure:: Structure(...){

...

pPoint = new Point*[numPlanes];

pPoint[j]=new Point[numPoints[j]];

// read in x,y,z coordinates in first loop
for (j=0;j<numPlanes;j++){
    for (k=0;k<numPoints[j];k++){

if (k==0){
pPoint[j][0].x = atof(strtok(sBuff,"\\"));
}

else{
pPoint[j][k].x = atof(strtok(NULL,"\\"));
}

pPoint[j][k].y = atof(strtok(NULL,"\\"));
pPoint[j][k].z = atof(strtok(NULL,"\\"));
...
    }
// calculate x,y coordinates for midpoint of plane j
midX[j]=(maxX[j]+minX[j])*0.5;
midY[j]=(maxY[j]+minY[j])*0.5;
}
// now loop over points again to determine radius at each point

for (j=0;j<numPlanes;j++){
    cout << "\nPlane " << j+1 << " of " << numPlanes << endl;

    for (k=0;k<numPoints[j];k++){
    	cout << "\nPoint " << k+1 << " of " << numPoints[j] << endl;

// setRadius method used to calculate radius
   	tempX = pPoint[j][k].x;
	tempY = pPoint[j][k].y;
	pPoint[j][k].setRadius(tempX,tempY,midX[j],midY[j]);//

// getRadius method returns calculated radius,r
	pPoint[j][k].r = pPoint[j][k].getRadius ();
    }
 }
}  // end of Structure constructor 

When compiling (g++ 4.3.2 [gcc-4_3-branch revision 141291] ) I receive no errors with the above code, but when executing a
segmentation fault occurs (SuSE Linux 11.1).

Output:

...
Midpoint of plane 22: (-1.1719,-172.64)

Plane 1 of 22

Point 1 of 152
Segmentation fault

As one can see from the output the values of numPlanes and numPoints[j] are
within scope - this was my first attempt at fixing the problem. I inserted the
following line

cout << " (" << pPoint[j][k].x << "," << pPoint[j][k].y << ")\n";

before the tempX=... line to test whether the problem lies with the
pPoint[j][k] object. Again I received a segmentation fault error when
executing (see output above).

And no this is not a homework question ...

My questions are:

0. What is wrong with the above code? The second loops lie within the
Structure constructor so pPoint[j][k] is still within scope.
1. Are the dummy variables "tempX", "tempY" really necessary? Is there a more
elegant way of achieving the same result? Or is it unwise to use the
pPoint[j]k].x and pPoint[j][k].y directly as a parameter in the setRadius
function call?
2. If not is there a better way to do this? Only the values of x_0 and y_0
have to be passed as arguments in the function call, but the function needs to
have at least a pointer to the current x and y values - pPoint[j][k].x and
pPoint[j][k].y.
This probably demonstrates a gap in my understanding of pointers ;-)

Thanks for taking the trouble to read this now rather long post. And thank you in advance to those replying.

[kbw]

A few points first.

Point::Point():x(0),y(0),z(0),r(0){
       x=y=z=r=0.0f;}

is initialising the member floats twice, first in the initialiser list and secondly in the function body.

Point::Point(float x, float y, float z, float r):x(x),y(x),z(z),r(r){
                    r=0.0f;}

Requires the user to provide a radius, but it's value is not used. First you set it to zero, then presumably call setRadius. The constructor should probabgly take x, y, z and call setRadius to initialise r.

Rather than manage arrays yourself, why not use a collection. std::vector seems well suited and removes some of the array management.

For example:

    // Private class member
    std::vector<Point*> m_points;

    // Constructor code
    std::ifstream infile("input.datafile");
    std::string line;

    m_points.resize(numPlanes);
    for (size_t i = 0; i < m_points.size(); ++i)
    {
        m_points[i] = new Point[numPoints[i]];

        std::getline(infile, line);
        std::istrstream is(line.c_str(), line.size());
        for (size_t j = 0; j < numPoints[i]; ++j)
            is >> m_points[i][j];
    }

    // Destructor code
    for (size_t i = 0; i < m_points.size(); ++i)
        delete [] m_points[i];

I've not compiled any of this, it's just to illustrate my point. I hope it helps.