Return to Answer

• 1

• 1.5k
• 3
• 19
• 27

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.