My Distance code

[pharoah85]

Could someone give me help on how to simplify this? Trying to write code to solve this equation:

Distance = sqrt((x2 - x1)^2 + (y2 - y1)^2 + (z2 - z1)^2)

I subbed a,b,c as the second set of coords. Im probably using too many vars as weel but im new, give it some time.

void school(int x,int y,int z, int a,int b,int c);

int main()
{int a,b,c,x,y,z;
cout<< "enter coordinates plz"<<endl;
cin>>x>>y>>z>>a>>b>>c;
school(x,y,z,a,b,c);

return 0;
}


void school(int x,int y,int z,int a,int b,int c)
{float d,e,f,g,h;

d=x-a;
e=y-b;
f=z-c;

g=powf(d,2)+powf(e,2)+powf(f,2);
h=sqrtf(g);
cout<<"the answer is "<<h;
}

[mike_2000_17]

It is generally wasteful to compute the square using the pow function, just do g = d*d + e*e + f*f; (you generally try to avoid using pow for integer powers (2,3,4,...)).

Then, the sqrtf is a non-standard function from C (which is provided by most implementations of math.h). You should, instead, use the C++ version. For this, use the function h = sqrt(g); from #include <cmath> instead of #include <math.h> (notice the "c" in front and no ".h" after, this is pretty much true for all the C libraries, although the math.h works like many other C libraries do, the versions that are written as cmath or cstdio or cstdlib (instead of math.h, stdio.h and stdlib.h) are more compatible with C++ code, and it might avoid you the pain of some crazy-looking compilation errors).

For the rest, it seems pretty ok. You are, of course, missing the #include headers that should go at the very start of your code:

#include <iostream>  //for the cout and cin
#include <cmath>     //for sqrt()
using namespace std; //to avoid having to write std:: in front of cout, cin, sqrt, etc.

[StuXYZ]

Well appart from the strange layout, it works. That is always good.

Do note that a*a is the same as pow(a,2).

It is also a lot more normal to do the calculations in the double form. E.g. pow(a,2.0); . It is often quicker as well, since the floating point unit is normally in double.

Finally, think about the fact that often the points are not on integer values, what if I want to test the point 0.5,0.5,1.0 ?

You would normally use tmp variables since it makes it clearer e.g. I would be inclined to write a function that returns a value, e.g. distance. That way you can say use if for determining if two spheres intersect.

double distance(// variables here )
{
const double xd=a-x;
const double yd=b-y;
const double zd=c-z;

return sqrt(xd*xd+yd*yd+zd*zd);
}

[pharoah85]

Thx. I can see the advantages of your tips. It still looks messy and I would like to learn how to write cleaner code. An example or two of how you would write code for the distance equation would be a big help.

[frogboy77]

Even after all that help it's still "do it for me"?

[mike_2000_17]

The reason why your code looks messy is mainly because your knowledge is still very limited. For example, here is how the solution would turn out if I wrote it with the code that I use on a day-to-day basis (omitting a few details, like the headers used and such):

int main() {
  vect<double,3> v1,v2;
  cout << "Enter coordinates please.." << endl;
  cin >> v1 >> v2;
  cout << "the answer is " << norm(v2 - v1);
  return 0;
};

But the above requires knowledge of numerous techniques in C++, which you will learn in proper time. Basically, the above uses a custom class template (called vect) that I wrote, which represents any fixed-size vector with a full suite of overloaded operators, helper functions and template specializations. But these are a bit too advanced to explain here or now. Be patient.

But besides that, there are several stylistic improvements you can make to the code that make it look a lot cleaner, here are a few I can think of just now:
- Use consistent indentation.
- Leave curly braces on their own lines.
- Declare variables on site of first use, if possible.
- Don't abuse local variables when they are not necessary and don't contribute to clarity.
- Give sensible names to variables.

Implementing these changes, your code turns into:

void print_distance(int x1, int y1, int z1, int x2, int y2, int z2);

int main()
{
  int x1, y1, z1, x2, y2, z2;
  
  cout << "Enter coordinates please.." << endl;
  cin >> x1 >> y1 >> z1 
      >> x2 >> y2 >> z2;
  
  print_distance(x1, y1, z1, x2, y2, z2);

  return 0;
}


void print_distance(int x1, int y1, int z1, int x2, int y2, int z2)
{
  double x_diff = x1 - x2;
  double y_diff = y1 - y2;
  double z_diff = z1 - z2;

  double dist = sqrt( x_diff * x_diff + y_diff * y_diff + z_diff * z_diff );
  cout << "the answer is " << dist << endl;
}

Doesn't this look cleaner?

For better style, you should be consistent with the types that you use, and you should prefer to write functions that don't have a mixed or double purpose (like computing the distance AND outputting it to the screen). Prefer function that have one clear purpose and use return values effectively. With this, it turns into:

double distance(double x1, double y1, double z1, 
                double x2, double y2, double z2);

int main()
{
  double x1, y1, z1, x2, y2, z2;
  
  cout << "Enter coordinates please.." << endl;
  cin >> x1 >> y1 >> z1 
      >> x2 >> y2 >> z2;
  
  cout << "The answer is " << distance(x1, y1, z1, x2, y2, z2) << endl;

  return 0;
}


double distance(double x1, double y1, double z1, 
                double x2, double y2, double z2)
{
  double x_diff = x1 - x2;
  double y_diff = y1 - y2;
  double z_diff = z1 - z2;

  return sqrt( x_diff * x_diff + y_diff * y_diff + z_diff * z_diff );
}

[pharoah85]

Thank you. Thats just what the doctor ordered Mike 2000 17. Its an inspiration. Ill strive to implement all the tips mentioned. Very clean code and understandable advice. Considered this thread solved. If I may ask one more thing. Being as knowledgeable as you are what book(s) would you recommend?