patterncppModerate
Finding the distance between two points in C++
Viewed 0 times
thepointsdistancetwobetweenfinding
Problem
I made a program in C++ where it calculates the distance between two coordinates. Is there anything to improve? Add? Make it more user-friendly?
Code:
Code:
#include
#include
using namespace std;
int ch;
double x;
double y;
double a;
double b;
double answer;
double distanceBetweenTwoPoints(double x, double y, double a, double b);
int main(){
cout > x;
cout > y;
cout > a;
cout > b;
cout << endl;
cout << endl;
answer = distanceBetweenTwoPoints(x, y, a, b);
cout << "The answer is " << answer;
}
double distanceBetweenTwoPoints(double x, double y, double a, double b){
return sqrt(pow(x - a, 2) + pow(y - b, 2));
}Solution
I think @john has made some excellent comments. I agree with them, but you've already gotten them, so I'll try not to repeat the same.
Global variables
Variables defined outside any function, like these:
...are generally called "global", because anything, anywhere in the world program can depend on them and/or modify them.
When some parts of the code modify them and other parts depend on them, it can become difficult to figure out how the code will act under any given circumstances, how to get it to to exactly what you really want, and so on.
Usually, you want to restrict each variable to (approximately) the smallest scope necessary for it to do its job. None of these variables is used outside of the
Avoid
Although it's used in a lot of tutorials and such,
Use functions
You may not have learned (much) about functions yet, but they can make your life much easier, largely by reducing repetition. For example, quite a bit of the code in main consists of a number of repetitions of the same basic sequence:
You can make your life considerably simpler by moving repeated actions like this into functions:
Then the code in
This not only reduces the amount of typing involved, but when you make a mistake (and we all do) it improves the chance that you've only made it in one place, so when you fix it, it'll be fixed in general.
Mathematics
You should usually think twice about computing a hypotenuse at all. In a lot of cases (e.g., comparing one distance to another) you can add the squares of the X/Y coordinates, but not take the square root of the result. For a lot of common cases like "find which point is closest to the one I clicked", the square of the distance works just as well as the actual distance, but is much faster to compute (
If you do need to compute a hypotenuse, consider using
Avoid
This
It's (by far) easier in the long term to bite the bullet and put up with typing things like
Note that this warning applies primarily (maybe even exclusively) to the
If you are going to have a
This can still lead to the same basic problems, but at least you've restricted the scope, so when a problem arises, cleaning up the mess is less work.
Global variables
Variables defined outside any function, like these:
int ch;
double x;
double y;
double a;
double b;
double answer;...are generally called "global", because anything, anywhere in the world program can depend on them and/or modify them.
When some parts of the code modify them and other parts depend on them, it can become difficult to figure out how the code will act under any given circumstances, how to get it to to exactly what you really want, and so on.
Usually, you want to restrict each variable to (approximately) the smallest scope necessary for it to do its job. None of these variables is used outside of the
main function, so we can define them there to assure that only the code in main can use them:int main(){
double x;
double y;
double a;
double b;
double answer;
cout > x;
cout > y;
cout > a;
cout > b;
cout << endl;
cout << endl;
answer = distanceBetweenTwoPoints(x, y, a, b);
cout << "The answer is " << answer;
}Avoid
std::endlAlthough it's used in a lot of tutorials and such,
std::endl is almost never what anybody really wants. Although it won't make any real difference as you've used it here, using std::endl when (for example) you're writing lots of data to a file can easily make your program run 10x slower than it really needs to1.Use functions
You may not have learned (much) about functions yet, but they can make your life much easier, largely by reducing repetition. For example, quite a bit of the code in main consists of a number of repetitions of the same basic sequence:
Ask user for input
Read the inputYou can make your life considerably simpler by moving repeated actions like this into functions:
double get_value(char const *prompt) {
double value;
std::cout > value;
return value;
}Then the code in
main to get values can be shortened a bit, to something like this:cout << "Enter the points for the coordinates\n";
x = get_value("Point x for first coordinate: ");
y = get_value("Point y for first coordinate: ");
a = get_value("Point x for second coordinate: ");
b = get_value("Point y for second coordinate: ");This not only reduces the amount of typing involved, but when you make a mistake (and we all do) it improves the chance that you've only made it in one place, so when you fix it, it'll be fixed in general.
Mathematics
You should usually think twice about computing a hypotenuse at all. In a lot of cases (e.g., comparing one distance to another) you can add the squares of the X/Y coordinates, but not take the square root of the result. For a lot of common cases like "find which point is closest to the one I clicked", the square of the distance works just as well as the actual distance, but is much faster to compute (
sqrt is often relatively slow).If you do need to compute a hypotenuse, consider using
std::hypot instead of re-implementing it yourself. In the worst case, this will just be a thoroughly tested and debugged version of the code you'd write on your own. In a better case, it might use a different algorithm to compute the result. For example, you might have a case where both your \$\Delta x\$ and \$\Delta y\$ are within range and the final answer would be within range, but \$\Delta x^2 + \Delta y^2\$ is out of range. In this case, the simplistic approach will overflow, but some others won't. Depending on your hardware and/or required precision, alternatives may also be faster.Avoid
using namespace std;This
using directive brings a huge number of names into scope, which can lead to all sorts of problems from colliding with names you try to define. Almost nobody really knows all the names it defines, so it's often hard to avoid them.It's (by far) easier in the long term to bite the bullet and put up with typing things like
std::cout when you need to use them.Note that this warning applies primarily (maybe even exclusively) to the
std namespace. Something like using namespace std::chrono; is often perfectly fine.If you are going to have a
using namespace std;, you usually want to restrict it to the scope of a single function:int main() {
using namespace std;
}This can still lead to the same basic problems, but at least you've restricted the scope, so when a problem arises, cleaning up the mess is less work.
- In case you care about the reason: making an operating system call to write data to a stream usually has a fair amount of overhead, so each call takes a fair amount of time, even to write a small amount of data. To avoid this, the code in the library stores data in a buffer. It only calls the OS to write the data to the external file once in a while, when the buffer gets full.
std::endl writes a new-line to Code Snippets
int ch;
double x;
double y;
double a;
double b;
double answer;int main(){
double x;
double y;
double a;
double b;
double answer;
cout << "Enter the points for the coordinates";
cout << endl;
cout << "Point x for first coordinates: ";
cin >> x;
cout << endl;
cout << endl;
cout << "Point y for first coordinate: ";
cin >> y;
cout << endl;
cout << endl;
cout << "Point x for the second coordinate: ";
cin >> a;
cout << endl;
cout << endl;
cout << "Point y for the second coordinate: ";
cin >> b;
cout << endl;
cout << endl;
answer = distanceBetweenTwoPoints(x, y, a, b);
cout << "The answer is " << answer;
}Ask user for input
Read the inputdouble get_value(char const *prompt) {
double value;
std::cout << "\n" << prompt;
std::cin >> value;
return value;
}cout << "Enter the points for the coordinates\n";
x = get_value("Point x for first coordinate: ");
y = get_value("Point y for first coordinate: ");
a = get_value("Point x for second coordinate: ");
b = get_value("Point y for second coordinate: ");Context
StackExchange Code Review Q#144586, answer score: 12
Revisions (0)
No revisions yet.