Point Structure

[HellfireXP]

I created this point structure that I'm pretty proud of. I was wondering if the professional programmers could take a look and let me know any ideas to improve it further. So far, I haven't found any bugs.

#ifndef POINT_H
#define POINT_H


struct point{
	public:
		/* The x and y variable are made PUBLIC because they are only limited by
		 * the float data type constraints.
		 */
		float x, y;
    
		/* The constructor will accept float and int data types or none at all.
		 * Two parameters sets to x,y respectively; one parameter sets both
		 * x and y to the same value.  An empty constructor sets x,y = 0.0
		 *
		 * There is no destructor since the point is essential just two floats
		 */
		point() : x(0.0), y(0.0) {};
		point(float x, float y) : x(x), y(y) {};
		point(float val) : x(val), y(val) {};
		point(int x, int y) : x(static_cast<float>(x)), y(static_cast<float>(y)) {};
		point(int val) : x(static_cast<float>(val)), y(static_cast<float>(val)) {};


		/* Returns the slope of two points.
		 * Formula for finding the slope(m) between 
		 * two coordinate points is: (y2-y1) / (x2-x1)
		 */
		float slope(point p) {
			return ((p.y-y) / (p.x-x));
		}


		/* Returns the distance between two points.
		 * Formula for finding the distance(d) between
		 * two points: square_root of [ (x2-x1)^2 + (y2-y1)^2 ]
		 */
		float distance(point p) {
			float d = ((p.x-x) * (p.x-x)) + ((p.y-y) * (p.y-y));
			return sqrt(d);
		}


		/* Increments the distance between two points.
		 * Formula for finding a new point between this point
		 * and point 'p' given a distance to increment:
		 * New(x) = x + (dist / square root of [ (m^2 + 1) ])
		 * New(y) = y + (m*dist / square root of [ (m^2 + 1) ])
		 */
		point increment(point p, float distance) {
			float m = slope(p);

			float newX = (distance / (sqrt((m*m) + 1))) ;
			newX = ((x < p.x) ? (x + newX) : (x - newX));

			float newY = ((m * distance) / (sqrt((m*m) + 1)));
			if (m >= 0.0) newY = ((y < p.y) ? (y + newY) : (y - newY));
			if (m < 0.0) newY = ((y < p.y) ? (y - newY) : (y + newY));

			return point(newX, newY);
		}


		//Returns the magnitude of a point.
		//Distance from point to (0,0)
		float magnitude() {
			return distance(point(0,0));
		}


		//Returns a normalized point
		point normalize() {
			return point(x/magnitude(), y/magnitude());
		}


		//Operators:

		//Addition
		point operator+=(point pnt){ (*this).x += pnt.x; (*this).y += pnt.y; return (*this); }
		point operator+=(float num){ (*this).x += num; (*this).y += num; return (*this); }
		point operator+(point pnt) { return point((*this).x + pnt.x, (*this).y + pnt.y);	}
		point operator+(float num) { return point((*this).x + num, (*this).y + num); }

		//Subtraction
		point operator-=(point pnt){ (*this).x -= pnt.x; (*this).y -= pnt.y; return (*this); }
		point operator-=(float num){ (*this).x -= num; (*this).y -= num; return (*this); }
		point operator-(point pnt) { return point((*this).x - pnt.x, (*this).y - pnt.y); }
		point operator-(float num) { return point((*this).x - num, (*this).y - num); }

		//Multiplication
		point operator*=(point pnt){ (*this).x *= pnt.x; (*this).y *= pnt.y; return (*this); }
		point operator*=(float num){ (*this).x *= num; (*this).y *= num; return (*this); }
		point operator*(point pnt) { return point((*this).x * pnt.x, (*this).y * pnt.y); }
		point operator*(float num) { return point((*this).x * num, (*this).y * num); }

		//Division
		point operator/=(point pnt){ (*this).x /= pnt.x; (*this).y /= pnt.y; return (*this); }
		point operator/=(float num){ (*this).x /= num; (*this).y /= num; return (*this); }
		point operator/(point pnt) { return point((*this).x / pnt.x, (*this).y / pnt.y); }
		point operator/(float num) { return point((*this).x / num, (*this).y / num); }

		//Equal (Assignment)
		point operator=(point pnt) { (*this).x = pnt.x; (*this).y = pnt.y; return (*this); }
		point operator=(float num) { (*this).x = num; (*this).y = num; return (*this); }

};

//Comparative operators:
bool operator==(point a, point b) { return a.x==b.x && a.y==b.y; }
bool operator==(point a, float num) { return a.x==num && a.y==num; }
bool operator!=(point a, point b) { return !(a==b); }
bool operator!=(point a, float num) { return !(a.x==num && a.y==num); }


//Other operators:
point operator+(float num, point pnt) { return (pnt + num); }
point operator*(float num, point pnt) { return (pnt * num); }


//Allows the display of the points in a 'coordinate' format (x,y) to the output stream
std::ostream& operator<<(std::ostream& os, const point a) {
	os << "(" << a.x << "," << a.y << ")";
	return os;
}


#endif 

[AbstractionAnon]

Line 11; x and y should be private. Using the principle of encapsulation means that you want to hide how a point is represented internally.

line 22,23: I would not bother with these constructors. The compiler will promote int arguments to floats automatically.

Lines 30,39,67,73,111-114: These functions should be const since they do not modify point.

Lines 87-106,118-119: These functions should return a reference.

Lines 81-106: this-> is generally preferred to (*this). Both do the same thing. It's really a matter of style.

In general, where point is passed as an argument, I prefer to pass by const reference rather than value. For a small class such as this it doesn't make much difference (pushing two floats verses pushing a pointer onto the stack). For larger classes, it can make a big difference in performance and is a good habit to get into.

[HellfireXP]

What would be the difference of making x and y public, verses something like:

private:
 float x, y;

public:
  float getX() const { return x; }
  void setX(float val) { x = val; }

The getter/setters in this case are literally just extra code. Since this is a point only restricted by the constraints of the data type float, isn't this a case where encapsulation would be unnecessary?

[AbstractionAnon]

What would be the difference of making x and y public, verses something like:

In a trivial example such as the point class above, there is essentially no difference.
However, it is still a good practice to hide member variables.

Consider the following contrived example:
Lets say the OP added a state variable to the class such as:

  bool is_origin;

Now, if x and y are modified all over the place in the code, then there is no assurrance that is_origin is being properly maintained. However, with a setter such as setX(), there is one point in the code that is responsible for maintaining is_origin.

is_origin would be better suited as a bool function, but as I said, this was a contrived example to show the impact of allowing public variables to be modified from outside the class

As for the "extra code", if the getters and setters are inlined, the functions will generally be optimized out by a good optimizing compiler.

The other advantage of using getters and setter is debugging. If public member variables are allowed to be modified willy nilly in a program, it is very hard to find where they might be being set incorrectly. By using a setter, you can set a breakpoint in the setter and catch everytime it is called.

[HellfireXP]

Thanks for the advise.. both responses. I've modified my code except for this:

Lines 87-106,118-119: These functions should return a reference.

I'm not sure how to do that? I tried return (&this); but that didn't work.

[AbstractionAnon]

I should have been more explicit.

The return type of those functions should be a reference type. Your return statements are correct.

point & operator += (point pnt)
{ (*this).x += pnt.x; (*this).y += pnt.y; return (*this); }