C++ code to find distance between line and point

Question

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 <vector>
#include <algorithm>
#include <iostream>

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, p3);
    std::cout << res << std::endl;
    return 0;
}

Answer 1

Include all headers

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

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.

Answer 2

Some remarks on your code

• you can use struct instead of class when you want to expose all the field members: it shows that you assume this exposition
• Do not expose public field members when you have class invariants. If an external user modifies Line::direction_vector and not Line::finish he/she will break your invariant without being aware of that. So, by default, move the field members of a class in the private area and use get/set methods for read/write access on them. Note that you can adopt a convention for private field members to gain an easy distinction with the arguments/access methods.
• I think you should avoid operator overloading if the signature is non-standard or if has (even minor) difference of what you naturally expect. Else external users of your code are likely to be confused or to take some times to understand errors.
• You can use const Vector& for method arguments instead of Vector. It will avoid some additional copies at least in the debug mode.
• Each struct/class is a consistent theory. So you can translate external functions into internal methods if they have sense for the class.
• Try to avoid comments like distance by using area of triangle is base*height / 2 and then calculate area using cross product. Just write the code in order that such a comment is no more useful by introducing meaningful intermediate variables.

Here are the remarks applied on your code

#include <vector>
#include <algorithm>
#include <iostream>

struct Point {
    int x;
    int y;

    Point (int x, int y) : x(x), y(y) {};
    Point() : x(0), y(0) {};
    Point (const Point&) = default;
};

struct Vector {
    int x;
    int y;

    Vector () : x(0), y(0) {};
    Vector (const Point& p1, const Point& p2)
      : x(p2.x - p1.x), y(p2.y - p1.y) {};
    Vector (const Vector&) = default;

    int getCrossProductAsInt(const Vector& source) const
      { return (x * source.y) - (y * source.x); }
    int getScalarProduct(const Vector& source) const
      { return (x * source.x) + (y * source.y); }
    double getNorm() const
      { return std::sqrt(x*x+y*y); }
};

class Line {
  private:
    Point m_start;
    Point m_finish;

  public:
    Line(const Point& st, const Point& fin) : m_start(st), m_finish(fin) {}
    Line(const Line&) = default;

    Vector getDirectionVector() const { return Vector(m_start, m_finish); }
    const Point& getStart() const { return m_start; }
    const Point& getFinish() const { return m_finish; }

    double getDistanceWith(const Point& point) const;
};

double
Line::getDistanceWith(const Point& point) const {
    Vector AB(m_start, m_finish);
    Vector AC(m_start, point); 
    double norm = AB.getNorm();
    double area = AB.getCrossProductAsInt(AC);
    return std::abs(area / norm);
}

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 = l1.getDistanceWith(p3);
    std::cout << res << std::endl;
    return 0;
}