Distance in Member Function

[marcchmiel96]

I'm having problems implementing the euclidean distance formula in the double distance member function. I've tried everything I can think of. Can someone help me with this?

#ifndef __POINT__
#define __POINT__
#include <iostream>
#include <math.h>
using namespace std;
class Point {
private:
	double x;
	double y;
public:
	Point() {
		x = 0.0;
		y = 0.0;

		return;
	}
	Point(double moreX, double moreY) {
		moreX = x;
		moreY = y;

		return;
	}
	Point(double moreX) {
		moreX = x;
		y = 0.0;

		return;
	}
	Point(const Point& p) {
		x = p.x;
		y = p.y;

		return;
	}
	const Point& operator=(const Point & rhs) {
		x = rhs.x;
		y = rhs.y;

		return rhs;
	}
	bool operator==(const Point p) const {
		return p == p.x && p == p.y;
	}
	bool operator!=(const Point p) const {
		return p != p.x && p != p.y;
	}
	double distance(const Point p) { //need help with this
		Point p1(p.x, p.y);
		Point p2(p.getX(), p.getY());
		
		return sqrt((p.getX() - p.x) * (p.getX() - p.x) + (p.getY() - y) * (p.getY() - y));
	}
	double getX() const {
		return x;
	}
	double getY() const {
		return y;
	}
	void setX(double newX) {
		x = newX;
	}
	void setY(double newY) {
		y = newY;
	}
};

istream & operator>>(istream & in, Point & p) {
	double x;
	double y;
	char seperator;
	in >> seperator >> x >> seperator >> y >> seperator;
	p.setX(x);
	p.setY(y);
	return in;
}
ostream & operator<<(ostream & out, const Point p1) {
	out << '(' << p1.getX() << ',' << p1.getY() << ')';
	return out;
}

#endif 

[Chervil]

Some of the constructors don't look right:

	Point(double moreX, double moreY) {
		moreX = x;
		moreY = y;

		return;
	}

assigning to moreX and moreY is the wrong way round, it is x and y which need to be initialised:

	Point(double moreX, double moreY) : x(moreX), y(moreY) { }

As for the distance function,

    double distance(const Point p)
    {
        return sqrt((p.x - x) * (p.x - x) + (p.y - y) * (p.y - y));
    }

[AbstractionAnon]

line 18-19,24: Your assignment statements are reversed. x and y should be on the left side.

line 51: p.getX() and p.x are the same value. Ditto for p.getY() and p.y, so all your terms are going to be zero.

Try this:

  return sqrt((getX() - p.x) * (getX() - p.x) + (getY() - y) * (getY() - y));

Lines 48,49: not needed.

edit: As keskiverto pointed out below, the getX() and getY() call are not required. I left them in place to highlight the difference from the OP's code.

[marcchmiel96]

I did not notice that constructor error, and I just looked back on the sample code th instructor gave us and your correct, and thanks for help with the distance formula, been racking my brain to figure out why it won't work.

[keskiverto]

Please describe the "problems". What do you see "going wrong"?

Your calculation does not use p1 or p2. Why are they there?

You don't need to call getX() and getY(), because member function has direct access to members and the get*() merely return member.

Let me rename parameter 'p' into 'other' and add some whitespace:

double
Point::distance( const Point other )
{
  Point p1( other.x,      other.y );
  Point p2( other.getX(), other.getY() );
		
  return sqrt(
    (other.getX() - other.x) * (other.getX() - other.x) +
    (other.getY() - y      ) * (other.getY() - y      )
  );
}

Do you now notice anything peculiar?

[marcchmiel96]

#include <iostream>
#include <string>
#include <fstream>
#include "__POINT__.h"

using namespace std;

int main() {
	const int MAX_NUM_POINTS = 1000;
	Point p[MAX_NUM_POINTS];
	Point distance;
	string fileName;
	ifstream infile;

	cout << "Enter file name: ";
	cin >> fileName;
	string pathName;

	infile.open(pathName + fileName);
	if (infile.fail()) {
		cout << "Can't open file " << fileName << endl;
		cout << "Aborting" << endl;

		return 1;
	}

	cout << "Reading in numbers." << endl;
	unsigned numPoints = 0;

	while (infile >> ws && !infile.eof() && numPoints < MAX_NUM_POINTS) {
		infile >> p[numPoints];
		numPoints++;
	}

	infile.close();

	cout << "Number of Points read is: " << numPoints << endl;
	cout << "Points read..." << endl;

	for (int i = 0; i < numPoints; ++i) {
		cout << p[i] << endl;
	}

	double minDistance = 100.0;
	double maxDistance = 0.0;

	double distanceTotal = 0.0;
	Point Point1;

	for (int i = 0; i < numPoints - 1; ++i) {
		for (int j = 1; j < numPoints; ++j) {
			distanceTotal = Point1.distance(p[j]);
			if (distanceTotal < minDistance) {
				minDistance = distanceTotal;
			} if (distanceTotal > maxDistance) {
				maxDistance = distanceTotal;
			}
		}
	}

	cout << minDistance << "\t" << maxDistance << endl;

	return 0;
}

My code along with my header file above does not print correct distance numbers, instead of being min is 1 and max is 10.2, it reads 4.2 and 11.9. I believe that my nested for loop is incorrect.

[keskiverto]

Why do you have a nested loop, when you only compare every point to (0,0)?
Besides, you don't calculate the the distance of p[0] from origin at all.

[marcchmiel96]

Oh thats what the loop is doing. I'm trying to compare all the points with each other to find the min and max distance of all the points. If that makes sense.

[keskiverto]

It could be more logical to have the distance() as standalone function, rather than as member. A distance between two points is the same as distance between this and that, but with a slightly different look and feel.

You could fill a N*N 2D matrix with the distances. On the diagonal the result is obviously 0.0, because that represents point to itself. The matrix is symmetric, so checking one triangle covers all cases. That is probably what you were trying.

Lets take a 4 point {A,B,C,D} case:
1 == The A's distance to B, C and D:

 ABCD
A.111
B
C
D

2 == The B's distance to C and D:

 ABCD
A.111
B .22
C
D

3 == C's distance to D

 ABCD
A.111
B .22
C  .3
D

4. D has already been located by all

 ABCD
A.111
B .22
C  .3
D   .

Can you figure out from that example how the row and column indices (counters of the two nested loops) should go?

(You don't need to store values in actual matrix; all you need to keep are the min and max.)


Edit: IIRC, either prefix or suffix double underscores are reserved for library use. It might be safer to use different names in the header inclusion guards.