HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

C++ code to find distance between line and point

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
linepointanddistancebetweenfindcode

Problem

So I made this simple program that allows users to construct points and lines and then return the smallest distance between a given point and a line. In doing so I have used some OOP concepts and operator overloading (both of which are fairly new territory for me). As such, I would really appreciate feedback. Furthermore, the code assumes that the user specifies a line by providing two distinct points and that this line is infinite.

```
#include
#include
#include

class Point {
public:
int x;
int y;
Point (int x, int y) : x(x), y(y) {};
Point() : x(0), y(0) {};
};

class Vector {
public:
int x;
int y;
Vector () : x(0), y(0) {};
Vector (Point p1, Point p2) : x(p2.x - p1.x), y(p2.y - p1.y) {};
};

class Line {
public:
Point start;
Point finish;
Vector direction_vector;
Line(Point st, Point fin) {
direction_vector = Vector(st, fin);
start = st;
finish = fin;
};
};

int operator^(const Vector& v1, const Vector& v2) {
int ret = ((v1.x) (v2.y)) - ((v1.y) (v2.x));
//cross product should return 3d vector but for convenience
//return an int (for example by dotting result with unit z vector)
return ret;
}

int operator*(const Vector& v1, const Vector& v2) {
int ret = ((v1.x) (v2.x)) + ((v1.y) (v2.y));
return ret;
}

double distance_line_point(Line l, Point p) {
Vector AB = l.direction_vector;
Vector AC(l.start, p);
int abs = ((AB)*(AB));
double norm = (double) std::sqrt(abs);
//return distance by using area of triangle is base*height / 2
//and then calculate area using cross product
double dist = ((double) (AB^AC)) / ((double) norm);
return std::abs(dist);
}

int main() {
Point p1(1, 1);
Point p2(2, 6);
//construct two points to form line
Point p3(3,3);
//construct point to find distance to line
Line l1(p1, p2);
//make line from two points
double res = distance_line_point(l1

Solution

Include all headers

Since you're using std::sqrt() and std::abs() you need to #include .

Line objects can be put in an invalid state

You seem to understand that a line requires two distinct points because you say


the code assumes that the user specifies a line by providing two distinct points

Still, it's probably better for your code to make sure start is not the same point as finish so that you can handle the error (e.g. by throwing an exception) as soon as you encounter it. This can make it easier to debug a program using your code -- as it is, I could construct a Line with the same two points and not notice later until the distance calculation returns a nonsensical result.

To make sure start != finish, make them private and provide getter and setter member functions. The setter functions should check that start and finish are not the same point. Both the setter functions and the constructor should check for this error condition.

Similarly, because all members of Line are public it's possible to break your distance calculation. Consider a slight modification to main():

int main() {
    Point p1(1, 1);
    Point p2(2, 6);
    //construct two points to form line
    Point p3(3,3);
    //construct point to find distance to line
    Line l1(p1, p2);

    // Evil
    l1.direction_vector = Vector(p2, p2);

    //make line from two points
    double res = distance_line_point(l1, p3);
    std::cout << res << std::endl;
    return 0;
}


The result is now -nan because distance_line_point() just uses Line::direction_vector without checking its validity. Not only do you need to check that Line::direction_vector uses the correct points defined by Line::start and Line::finish, you need to check that the Vector is non-zero.

The solution would be the same as before: make Line::direction_vector private and provide getter and setter functions for it (or perhaps just a getter, and re-calculate it when start or finish is modified). The setter functions for all three members of Line would need to ensure the validity of the other two.

That said...

Line should not have the member direction_vector

Two (distinct) points are all that is required for a line. You seem to be providing Line::direction_vector just so that distance_line_point() can determine the vector between the two points that define the Line. However, you can easily calculate that vector in distance_line_point(). With Line defined as you have it, you can replace

Vector AB = l.direction_vector;


with

Vector AB(l.start, l.finish);


Now you can remove Line::distance_vector.

Operator overloading

See C++ Operator overloading on Stack Overflow for general guidelines on how to overload operators. You seem to be following those guidelines pretty well for the operators you've overloaded.

However, consider adding some additional overloads that make sense. For example, these operators would come in handy if you modified Line to make sure that its two points are not the same:

inline bool operator==(const Point& lhs, const Point& rhs) {
    return lhs.x == rhs.x && lhs.y == rhs.y;
}

inline bool operator!=(const Point& lhs, const Point& rhs) {
    return !operator==(lhs, rhs);
}


Also, since you've defined for your Vector class it would make sense to define the other operators that make sense for vectors: =, +, +=, etc. I would be surprised to be able to multiply two Vector objects but not add them.

Code Snippets

int main() {
    Point p1(1, 1);
    Point p2(2, 6);
    //construct two points to form line
    Point p3(3,3);
    //construct point to find distance to line
    Line l1(p1, p2);

    // Evil
    l1.direction_vector = Vector(p2, p2);

    //make line from two points
    double res = distance_line_point(l1, p3);
    std::cout << res << std::endl;
    return 0;
}
Vector AB = l.direction_vector;
Vector AB(l.start, l.finish);
inline bool operator==(const Point& lhs, const Point& rhs) {
    return lhs.x == rhs.x && lhs.y == rhs.y;
}

inline bool operator!=(const Point& lhs, const Point& rhs) {
    return !operator==(lhs, rhs);
}

Context

StackExchange Code Review Q#143424, answer score: 4

Revisions (0)

No revisions yet.