File I/O, classes

I 'm having trouble with a part of my program that reads in points (coordinates) from a file and then prints them out. It's spitting out garbage. I think it has to do with the printing rather than the scanning. I am new to c++.

Here's a snippit from my main.cpp:

Code:

---

ifstream isIn("points.txt", ios::in);

i=0;
Point2D[i].setX(isIn, dX);  //scanning in first X coordinate
Point2D[i].setY(isIn, dY);  //scanning in first Y coordinate
i++;
while(!isIn.eof() && i < MAXNUMPTS)
{
      Point2D[i].setX(isIn, dX);  //scanning the remaining X coordinates
      Point2D[i].setY(isIn, dY);  //scanning the remaining Y coordinates
      i++;
}

cout <<"Here are your coordinates:" <<endl;
for (j=0; j<(i+1); j++)
{
      cout <<"Point "<< j+1;
      dX = Point2D[j].getX();  //Printing all X coordinates
      cout <<dX;
      dY = Point2D[j].getY();  //Printing all Y coordinates
      cout <<dY;
      cout <<")\n";
}

---

And here's a snippit from another .cpp file that's included (function implimentations):

Code:

---

void Point::setX(ifstream& isIn, double dX)
{
    isIn >> dX;
}
             
double Point::getX()
{
      return(dX);
}
             
void Point::setY(ifstream& isIn, double dY)
{
        isIn >> dY;
}
             
double Point::getY()
{
      return(dY);
}

---

>> for (j=0; j<(i+1); j++)
That should be j < i, since i+1 hasn't been set yet.

>> while(!isIn.eof()
You shouldn't use eof() to control an input loop. Change your set functions to return istream& and then use that as a bool. Or you could just use if (!isIn) break; after attempting to read in X so that the loop breaks if the end of file is reached.

Those probably aren't the problem though. How do you declare Point2d? Do you allocate space for it? Do you do any other reading from isIn? What does the input file look like?

NOTE: Echo's some of what Daved already said.

Code:

---

while(!isIn.eof() && i < MAXNUMPTS)

---

Beware using eof to control the execution of your loops. I made a recent post on the whys/why nots.

Code:

---

void Point::setX(ifstream& isIn, double dX)
{
    isIn >> dX;
}
             
void Point::setY(ifstream& isIn, double dY)
{
    isIn >> dY;
}

---

Why are you passing in a double argument to this function? You only need to set the classes member variable and for that you don't need to pass in anything. I would scrap these two functions completely. Generally I like to overload the stream operators (<< and >>) for the class to make things more natural looking:

Code:

---

class Point
{
    ...
public:
    ...
    friend ostream& operator<<(ostream&, const Point&);
    friend istream& operator>>(istream&, Point&);
};

istream& operator>>(istream& is, Point& pt)
{
    return is >> pt.dX >> pt.dY;
}

ostream& operator<<(ostream& os, const Point& pt)
{
    return os << pt.dX << ' ' << pt.dY;
}


...

// Read up to MAXNUMPTS from file into Point2D array.
i = 0;
while( (i < MAXNUMPTS) && (isIn >> Point2D[i]) ) ++i;

// Write the points back to console.
for( j = 0; j < i; ++j )
{
    cout << "Point " << j+1 << " = " << Point2D[j] << endl;
}

---

Those first things Dave said don't seem to be contributing to the problem.

Point2D is declared:

Code:

---

Point Point2D[MAXNUMPTS];

---

I'm not sure about your new code, it seems rather advanced for me, we have not covered that yet.

And I was also told to use the functions that I have declared. I'm trying to understand what's going wrong.

If I had to guess, I'd say your problem is that you're passing doubles to your functions. As I'd imagine, dX and dY are members of your class. Since you're using the same name for the arguement, and since it's not a reference, you're creating an instance of a new variable dX and dY. One in the scope of the function. One in the scope of the class. What you're reading into is the function variable, which is then destroyed when you leave the function scope.

Just delete those double arguements and make them private class members if they aren't already. Class functions can access class members without them being passed to the function.

You'd also get correct results simply by qualifying the variables with the class name:

Code:

---

isIn >> Point::dX;

---

...but it's just silly to pass the member to the function, anyway.

Just delete those double arguements and make them private class members if they aren't already. Class functions can access class members without them being passed to the function.

You are right, dX and dY are private members of my class. So I'm just trying to understand what you are saying.

Where I have this:

Code:

---

Point2D[i].setX(isIn, dX);

---

I should change do something like:

Code:

---

Point2D[i].setX(isIn, dXtoRead);

---

But where I have this:

Code:

---

void Point::setX(ifstream& isIn, double dX)
{
    isIn >> dX;
}

---

should I change to this?:

Code:

---

void Point::setX(ifstream& isIn, double dXtoRead)
{
    isIn >> dXtoRead;
    dX = dXtoRead;
}

---

What's the point of the extra variable?!? :D :D

Like this:

Code:

---

void Point::setX(ifstream& isIn)
{
    isIn >> dX;  // This is the member of the class. You can access it from here.
}

---

and the function call:

Code:

---

Point2D[i].setX(isIn);

---

It's that simple. Now if you know how to overload the insertion and extraction operator as hpk said, then try that, otherwise do the functions.

Yea OK that makes sense, I don't know why I thought I had to have those there.

I'll have to try the program when i'm back on my other computer and see how she goes.

Thanks.