# Simple C++ program that calculates the measures of a sphere

I have another home work from school and would like to see if I am on the right path to learning C++.

We have to do the calculation of the measures of a sphere. I did it two ways:

1. Just calculating everything at the main
2. Using a function

#include "stdafx.h"
#include "iostream"
#include <cmath>

double *calcSphereMesures(double r);

double *calcSphereMesures(double r)
{
const double PI = 3.1416;
double d, area, circumference, volume;
double results[4];

d = 2*r;
circumference = PI * d;
area = (4 * PI * pow(r,2));
volume = (4 * PI * pow(r,3))/3;

results[0] = area;
results[1] = circumference;
results[2] = volume;
results[3] = d;
return results;
}

int main()
{
double r;
std::cin >> r;
double *results = calcSphereMesures(r);

std::cout << "\n**************************************" << "\n"
<< "* Area and Circumference of A Sphere *" << "\n"
<< "**************************************" << "\n"
<< "\nRadius: " << r << "\n"
<< "Diameter: " << results[3] << "\n"
<< "Area: " << results[0] << "\n"
<< "Circumference: " << results[1] << "\n"
<< "Volume: " << results[2];
return 0;
}


I tried to check all the tips that were given to me the last time. Is that still bad code? What can I improve here and remember for future exercises?

• Since you are using C++ you could have made a class for Sphere and make get and set methods. Get methods like getArea(), getVolume(), etc. In the Driver you will need to create an object named Sphere. Object oriented codes looks better, cleaner and simpler. Commented Sep 12, 2017 at 12:40
• @Ugnes Please do not answer in comments. All suggestions for improving the code belong in answers. Commented Sep 12, 2017 at 14:28

## Prefer portable code

This code could be entirely portable if the #include "stdafx.h" were omitted. It's Windows specific and doesn't really help here, even on Windows.

There is a subtle difference between #include "iostream" and #include <iostream>. Although it's implementation defined, most compiler implementations is that the quotation mark form searches locally first (e.g. the current directory) and then searches system include directories if that fails. The angle bracket form usually searches in system include directories. See https://stackoverflow.com/questions/21593/what-is-the-difference-between-include-filename-and-include-filename for more.

The code is all jammed up on the left margin with no indenting. I don't know if that's how you wrote it or if it's a cut-and-paste error when posting the question, but it really makes the program hard to read. It makes no difference to the compiler, but you should strive for a good consistent style that includes visual clues for the human reader of your code.

## Fix the bug

The calcSphereMesures routine is returning the address of a variable that's local to that routine. That's a bug. If you need to return multiple values, either pass in a pointer to an appropriate structure to be filled in by the routine or allocate one using new and returning that. I'd prefer the former, because it makes ownership of the structure a little cleaner (that is, the caller must both allocate and free the structure). Note that although I'm using the word structure, here, that isn't necessarily a struct -- it could just as well be an array or some other data structure.

## Consider using an object

The area, circumference, etc., could all be attributes of a Sphere class. This would have the advantage that one could write sphere.diameter() instead of results[3] for example.

## Use name spaces

When you #include <cmath>, it brings those functions into the std:: namespace and may also put the symbols into the global namespace. You're counting on the latter which is not guaranteed by the standard. You can fix that by using std::pow instead of just pow.

## Use a better value of $\pi$

There's little point in asking for the precision of a double if you're going to approximate $\pi$ as 3.1416.

## Understand the use of function declarations

A function declaration is required before the use of the function. Your separate declaration and implementation of the calcSphereMesures() function would make sense if the declaration were in its own header file, but that's not the case here. For that reason, I'd suggest eliminating the declaration -- keeping them synchronized is just another task that you can eliminate.

The word "measure" is consistently misspelled within the code. It would be nice to fix that.

• I believe he returns local array as a pointer, which is UB, is it correct? That is why I close voted Commented Sep 12, 2017 at 21:37
• @Incomputable: yes, it's undefined behavior, but depending on compiler and OS, the code might well appear to work. Commented Sep 13, 2017 at 1:08
• I am using the VS and if I do not include the stdafx.h library, the code does not compile. Commented Sep 15, 2017 at 12:58
• You can turn off precompiled headers and VS will compile it just fine. Commented Sep 15, 2017 at 13:27

As mentioned by Edward, bug-less version would look somewhat like this:

void calcSphereMeasures(double[] results, double r)
{
const double PI = double(3.1415926535897932384626433832795);
double d, area, circumference, volume;

d = 2*r;
circumference = PI * d;
area = (4 * PI * pow(r,2));
volume = (4 * PI * pow(r,3))/3;

results[0] = area;
results[1] = circumference;
results[2] = volume;
results[3] = d;
}

int main()
{
...
double results[4]; // memory remains in this scope
calcSphereMeasures(results, r);
std::cout << results[0];
...
}


Yet it is considered unsafe as you can pass, for example, 0 as first parameter, and that will result in attempt to write to inacessible memory.

This, on the other hand, is a more readable and safe version:

struct SphereMeasures
{
double area;
double circumference;
double volume;
double d;
};

SphereMeasures calcSphereMeasures(double r)
{
const double PI = double(3.1415926535897932384626433832795);
SphereMeasures result;

result.d = 2*r;
result.circumference = PI * result.d;
result.area = (4 * PI * pow(r,2));
result.volume = (4 * PI * pow(r,3))/3;

return result;
}

int main()
{
...
SphereMeasures result = calcSphereMeasures(r);
std::cout << result.area;
...
}


It is miles far from clean popular solutions, but it is a start.

• Could you fix the two typos in the struct and funcion names, please? Also, what's the point of the double in double(3.141...)? Commented Sep 13, 2017 at 7:28
• @RolandIllig , thanks, seems to be good now. No actual point in using double(val), just the preference. Commented Sep 13, 2017 at 10:18

I think you could provide hours of joy to your HW assignment checker if you implement something based on the 2D case (a disc), which has the following charming solution:

#define _ -F<00||--F-OO--;
int F=00,OO=00;main(){F_OO();printf("%1.3f\n",4.*-F/OO/OO);}F_OO()
{
_-_-_-_
_-_-_-_-_-_-_-_-_
_-_-_-_-_-_-_-_-_-_-_-_
_-_-_-_-_-_-_-_-_-_-_-_-_-_
_-_-_-_-_-_-_-_-_-_-_-_-_-_-_
_-_-_-_-_-_-_-_-_-_-_-_-_-_-_
_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_
_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_
_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_
_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_
_-_-_-_-_-_-_-_-_-_-_-_-_-_-_
_-_-_-_-_-_-_-_-_-_-_-_-_-_-_
_-_-_-_-_-_-_-_-_-_-_-_-_-_
_-_-_-_-_-_-_-_-_-_-_-_
_-_-_-_-_-_-_-_
_-_-_-_
}


Note that this scales very well and in fact gets even more accurate as you scale it!

For making this 3-D, consider measuring slices of the sphere at different latitudes - which gradually increase in size up to the equator. Another fun challenge :-)

Source: IOCCC 1988.

• Interesting - but in how far does this answer constitute a code review of the question? Commented Sep 12, 2017 at 23:06
• @ChrisWue: It suggests an alternative approach to that taken by the author in his/her code. Commented Sep 12, 2017 at 23:08