Hello all,

I'm new to programming and I came across the practice problems thread and tried one of the beginner ones:
"Make a program that allows the user to input either the radius, diameter, or area of the circle. The program should then calculate the other 2 based on the input."

I tried it out and it seems to work.
Is there any way this can be improved or made more efficient and is there any unneeded code?

Thanks for your help.

// find radius, diameter, or area using one of the three

#include <cstdio>
#include <cstdlib>
#include <iostream>
#include <cmath>
using namespace std;

void displayInstructions()
{
    cout << "Enter one of the following for a circle:\n"
         << "Radius, diameter, or area.\n"
         << "For unknowns, use 0" << endl;
}

void findCircleMeasurements()
{
    double nRad, nDia, nArea; // set variables in circle
    cout << "Enter Radius: ";
    cin >> nRad;
    cout << "\nEnter Diameter: ";
    cin >> nDia;
    cout << "\nEnter Area: ";
    cin >> nArea;
    cout << "\n";

    if(nArea != 0)
    {
        nRad = sqrt(nArea/3.14);         // radius == sqrt of area over pi
        nDia= 2*nRad;                   // diameter == 2*radius
        cout << "Radius is "
             << nRad
             << "\nDiameter is "
             << nDia;
             return;
    }
    if(nRad != 0)
    {


             nDia = 2*nRad;             // diameter == radius*2
             nArea = pow(nRad,2)*3.14;  // area == radius squared times pi

        cout << "Diameter is "
             << nDia
             << "\nArea is "
             << nArea;
             return;
    }
    if(nDia != 0)
    {

        nRad = nDia/2;                // radius == diameter/2
        nArea = pow(nRad,2)*3.14;     // area == radius squared times pi
        cout << "Radius is "
             << nRad;
        cout << "/nArea is "
             << nArea;
             return;
    }
}
int main( int nNumberofArgs, char* pszArgs[])
{
displayInstructions(); // execute the two functions
findCircleMeasurements();


cin.get();
return 0;
}

the only thing i would've changed about your code was using the endl command instead of the \n

cout << "output" << endl;

the only thing i would've changed about your code was using the endl command instead of the \n

I wouldn't. It's slower.

Is there any way this can be improved or made more efficient and is there any unneeded code?

Well if you look at your code, you can see that your typing the same piece of code a few times, so perhaps make a function that calculates the diameter and on for the radius?

Several things come to mind. [and in no particular order]. You will have to judge what you think the importance is.

(a) You test a double precision number to be exactly equal to zero, that very often doesn't happen, e.g. your get 1e-314 or something very close to zero but not zero. In many programs you should NEVER test for zero but rather being within a tolerance,e.g. fabs(Var)<1e-20) .

(b) define a constant for pi, [or use M_PI from cmath] with a lot more significant figures, you can also use 4.0*atan(1) , or you favorite trig identity.

(c) On the entry part, if the user enters a radius, then don't ask for an area or a diameter. At the moment if they input all three, then the area dominates.

(d) The output part [as pointed out by Nick Evan] is repeated, so why not use an
if else construction and put the output at the end.

(e) pow(x,2) is equivalent to x*x. I would write the later, BUT my compiler g++, actually produces the same machine code so it is purely stylistic.

(f) There is no test for negative entry.

(g) not putting a '\n' on the last output line seems a bit ugly

(h) you have written int main() as int main( int nNumberofArgs, char* pszArgs[]) , it is very common to write that as int main(int argc,char* argv[]) . So common that it is almost a standard.


Long list (sorry). For a school project, I guess if you do a few of them, it normally impresses, although to my mind you have grasped the basics and shouldn't worry and just do the next more interesting problem.


p.s. Excellent first post! Thanks for reading the posting-FAQ.

Edited 5 Years Ago by StuXYZ: n/a

Thanks for all your responses.

I see how I could implement those changes and they make sense. But seeing as how this code is simple and I'd rather move on to more advanced techniques, I think my time will be better spent on other things.

This article has been dead for over six months. Start a new discussion instead.