3d distance

Started by
4 comments, last by Zakwayda 13 years, 7 months ago
Would someone look this short piece of code over please. The assignment comes from a book I have and it wants you to find the distance between objects in a 3d environment. I'm posting this because I would like someone to see if the code makes sense. The program contains the first function I've ever written (aside from main) and I've declared a lot of variables in said function. Let me know. Thanks.

#include <iostream>#include <cmath>using namespace std;float dist3(float ux, float uy, float uz, float vx, float vy, float vz);float powf(float x, float y);float sqrtf(float x);float total = 0;int main(){dist3(1, 2, 3, 0, 0, 0);cout << "The distance between (1, 2, 3) and (0, 0, 0) is ";cout << total << endl;dist3(1, 2, 3, 1, 2, 3);cout << "The distance between (1, 2, 3) and (1, 2, 3) is ";cout << total << endl;dist3(1, 2, 3, 7, -4, 5);cout << "The distance between (1, 2, 3) and (7, -4, 5) is ";cout << total << endl;int response;cin >> response;return 0;}float dist3(float ux, float uy, float uz, float vx, float vy, float vz){float y = 2;float set1 = vx - ux;float set2 = vy - uy;float set3 = vz - uz;float s1power = 0;float s2power = 0;float s3power = 0;float sum = 0;s1power = powf(set1, y);s2power = powf(set2, y);s3power = powf(set3, y);sum = (s1power + s2power + s3power);total = sqrtf(sum);return 0;}
Advertisement
float dist3(float ux, float uy, float uz, float vx, float vy, float vz)
{
float dx = ux - vz;
float dy = uy - vy;
float dz = uz - vz;

return (float) sqrt( (dx * dx) + (dy * dy) + (dz * dz) );
}
Quote:Original post by verbalabuse
Would someone look this short piece of code over please. The assignment comes from a book I have and it wants you to find the distance between objects in a 3d environment. I'm posting this because I would like someone to see if the code makes sense. The program contains the first function I've ever written (aside from main) and I've declared a lot of variables in said function. Let me know. Thanks.

*** Source Snippet Removed ***
Yes, your code appears to be correct (except for the fact that you're returning '0' rather than the total, which is presumably just an oversight). As GregMichael pointed out though, it can be written more concisely.

A couple of things worth noting:

1. std::pow() is usually overkill if you just need to square a number; just multiply it by itself instead.

2. Declaring all the variables up front like that is an old C-style convention, and is not necessary or desirable in C++ (or in C99). Just declare them at the point of first use.
Thank you for the help.
Using a global variable to return the result of the function is a very bad idea. Do what others have suggested, and then in the calling code do
cout << "The distance between (1, 2, 3) and (0, 0, 0) is ";cout << dist3(1, 2, 3, 0, 0, 0) << endl;


Quote:Original post by jyk
1. std::pow() is usually overkill if you just need to square a number; just multiply it by itself instead.


I was surprised to find out recently that g++ generates the same assembly output in both cases. Not only that but if the exponent is a larger natural number std::pow will generate a close-to-optimal sequence of multiplications to compute it.

So although I would have given you the same advice a few months ago, now I think this one falls in the "do whatever is cleaner" category.

Quote:Original post by alvaro
I was surprised to find out recently that g++ generates the same assembly output in both cases. Not only that but if the exponent is a larger natural number std::pow will generate a close-to-optimal sequence of multiplications to compute it.

So although I would have given you the same advice a few months ago, now I think this one falls in the "do whatever is cleaner" category.
It doesn't surprise me that a compiler might make that optimization. But, x*x is certainly unlikely to be any *slower* than calling std::pow(), so it seems to me x*x would probably still be the safer bet. (Granted though, it's unlikely to matter either way in most cases.)

This topic is closed to new replies.

Advertisement