help with getting functions to work together

ok i posted something similar earlier but i've refined it and it works almost how i want, but not quite there. i know the code is messy but that's not the issue right now. i'm trying to set a menu so that a user can choose 4 shapes to calculate the surface area of. i have to use functions for the menu, the surface area, and the "getvalue" thing. that was giving me issues before but i've gotten that working. so the problem i'm having now is that when i run the program, the menu will display, and will display again after each shape, but the input for the menu does nothing. so if sphere is 1, prism is 2, cone is 3, pyramid 4, entering any of those does nothing. and entering 5 doesn't exit my while loop. it just runs through the shapes 1, 2, 3, 4 and then back to the top. what am i doing wrong? i just need to get the menu function, and the number the user inputs in there to work with the other functions in the program.

Code:

#include <iostream>
#include <iomanip>
#include <string>
#include <math.h>
using namespace std;
const double PI = 3.14159;
int value,choice, radius;
float surface_area_sphere, surface_area_prism, surface_area_cone, surface_area_pyramid;



int menu()
{
cout<<"\nSurface Area Calculator" 
<<"\n1) Sphere"
<<"\n2) Cone"
<<"\n3) Rectangular Prism"
<<"\n4) Pyramid"

<<"\n\n5) Quit"
<<"\n\nEnter Your Choice: ";
cin>>choice;
while (choice<1 || choice>5)
{
cout<<"\nInvalid choice, please try again";
cin>>choice;
}

return(0);
}




float calcSphere (int radius)
{

surface_area_sphere=(4*PI*radius*radius);
cout<<"The surface area of the sphere is"<<fixed<<setprecision(4)<<surface_area_sphere;
return (surface_area_sphere);
}




int getvalue (string prompt, int lowerBound, int upperBound)
{
cout<<prompt<<lowerBound<<" - "<<upperBound;
cin>>value;
while (value<lowerBound || value>upperBound)
{
cout<<"invalid number, please try again";
cin>>value;
return (value);
}
}



float calcPrism (int length, int width, int height)
{
surface_area_prism = ( 2 * length * width ) + ( 2 * width * height ) +
                                    ( 2 * length * height );
cout<<"\nThe surface area of the prism is:"<<fixed<<setprecision(4)<<surface_area_prism;
return(surface_area_prism);	   	   	   	   	   	   	   	   	   
}



float calcCone(int radius, int length)
{
surface_area_cone = ( PI * radius * length ) + ( PI * radius*radius );
cout<<"\nThe surface area of the cone is:"<<fixed<<setprecision(4)<<surface_area_cone;
return (surface_area_cone);
}



float calcPyramid (int length, int height)
{
surface_area_pyramid = (2*length*height) + (length*length);
cout<<"\nThe surface area of the pyramid is: "<<fixed<<setprecision(4)<<surface_area_pyramid;
return (surface_area_pyramid);
}


int main()
{

while (choice !=5)
{
menu();


if (choice=1) 
{
value=getvalue("\nenter the radius of the sphere", 1, 10);

calcSphere (value) ;
}
menu();
if (choice=2)
{
value=getvalue("\nenter the length of the prism", 1, 20);
value=getvalue("\nenter the width of the prism", 1, 20);
value=getvalue("\nenter the height of the prism", 1, 20);
calcPrism (value, value, value);

}
menu();
if (choice=3)
{
value=getvalue("\nenter the radius of the cone", 1,7);
value=getvalue("\nenter the length of the cone", 1, 12);
calcCone (value, value);
}
menu();
if (choice=4)
{
value=getvalue("\nenter the length of the pyramid", 1, 10);
value=getvalue("\nenter the height of the pyramid", 1, 15);
calcPyramid (value, value);
}
}
return 0;
}

First of all, messy code is always an issue. If your code is messy, this hurts your ability to find a fix bugs (such as the one you're struggling with now).

The comparison operator is "==", not "=".

yeah i know, but i'm able to keep track of what's happening and just discovered some other issues with my code, so i need to fix those up now. and yeah i know about the == but i was experimenting since the menu function wasn't working for me. thanks.

In your code there is no need to call the menu() function again and again. You are checking whether the choice is 1 and then you are calling menu() and then you are checking whether the choice is 2 and then you are calling the menu() function. Why that has to be? Any way until you are giving choice 5 it will call the menu() function. So there is no need of calling the menu() function again and again

here is my revised code, again still with the same issue with menu, but i fixed some other issues

Code:

#include <iostream>
#include <iomanip>
#include <string>
#include <math.h>
using namespace std;
const double PI = 3.14159;
int value,choice, radius, length, width, height;
float surface_area_sphere, surface_area_prism, surface_area_cone, surface_area_pyramid;



int menu()
{
cout<<"\n\nSurface Area Calculator" 
<<"\n1) Sphere"
<<"\n2) Cone"
<<"\n3) Rectangular Prism"
<<"\n4) Pyramid"

<<"\n\n5) Quit"
<<"\n\nEnter Your Choice: ";
cin>>choice;
while (choice<1 || choice>5)
{
cout<<"\nInvalid choice, please try again: ";
cin>>choice;
}

return(choice);
}




float calcSphere (int radius)
{

surface_area_sphere=(4*PI*radius*radius);
cout<<"The surface area of the sphere with radius "<< radius<<" is: "<<fixed<<setprecision(4)<<surface_area_sphere;
return (surface_area_sphere);
}




int getvalue (string prompt, int lowerBound, int upperBound)
{
cout<<prompt<<lowerBound<<" - "<<upperBound;
cout<<"\n";
cin>>value;
while (value<lowerBound || value>upperBound)
{
cout<<"\nInvalid number, please try again: ";
cin>>value;
return (value);
}
}



float calcPrism (int length, int width, int height)
{
surface_area_prism = ( 2 * length * width ) + ( 2 * width * height ) +
                                    ( 2 * length * height );
cout<<"\nThe surface area of a prism with length "<<length<<" and width "<<width<<" and height "<<height<<" is: "<<fixed<<setprecision(4)<<surface_area_prism;
return(surface_area_prism);	   	   	   	   	   	   	   	   	   
}



float calcCone(int radius, int length)
{
surface_area_cone = ( PI * radius * length ) + ( PI * radius*radius );
cout<<"\nThe surface area of a cone with radius "<<radius<<" and length "<<length<<" is: "<<fixed<<setprecision(4)<<surface_area_cone;
return (surface_area_cone);
}



float calcPyramid (int length, int height)
{
surface_area_pyramid = (2*length*height) + (length*length);
cout<<"\nThe surface area of a pyramid with length "<<length<<" and height "<<height<<" is: "<<fixed<<setprecision(4)<<surface_area_pyramid;
return (surface_area_pyramid);
}


int main()
{

while (choice !=5)
{
choice=menu();


if (choice=1) 
{
radius=getvalue("\nEnter the radius of the sphere: ", 1, 10);

calcSphere (radius) ;
}
choice=menu();
if (choice=2)
{
length=getvalue("\nEnter the length of the prism: ", 1, 20);
width=getvalue("\nEnter the width of the prism: ", 1, 20);
height=getvalue("\nEnter the height of the prism: ", 1, 20);
calcPrism (length, width, height);

}
choice=menu();
if (choice=3)
{
radius=getvalue("\nEnter the radius of the cone: ", 1,7);
length=getvalue("\nEnter the length of the cone: ", 1, 12);
calcCone (radius, length);
}
choice=menu();
if (choice=4)
{
length=getvalue("\nEnter the length of the pyramid: ", 1, 10);
height=getvalue("\nEnter the height of the pyramid: ", 1, 15);
calcPyramid (length, height);
}
}
return 0;
}

the issue is that the input into the menu function isn't calling the correct function. no matter what i enter intiially, i get the sphere, no matter what i enter after that, i get the prism, and entering 5 is not breaking the loop.

and to neon black, interchanging = and == isn't making any difference at the current moment, so something else is going wrong before that i guess.

Code:

if (choice=1) 
...
if (choice=2)
...

Remember what I said about = and == ? This is what I was talking about.

Yes it is making a difference. You just aren't paying close enough attention.

Go though your main function on paper (or typing). Write out the program flow step by step.

You have some serious logic issues.

ok i've got it working. thanks for the help everyone, i appreciate it very much.

You are making a big mistake thinking that there is something more important to do to this code beyond indent it. The absolute #1 thing I would do first with this if I had to work on it would be to indent it.

This code could be so full of bugs that indenting it is a waste of time, because you might as well just throw it out. However, hopefully that is not the case, so you might as well indent it properly now.

Also, keeping lines down to a few hundred characters is a good idea.

You contention that "i'm able to keep track of what's happening" is TOTALLY IRRELEVANT. Look at it this way: You will be changing your mind (unless you give up programming in the next week or so). After that, you will write indented code with reasonable length lines.

So -- HEY -- let's start right now. Why not?

Code formatting is the number one way to avoid simple errors. Don't ever underestimate the value of well formatted code. When looking at older code that isn't quite formatted the way I'm used to seeing it the first thing I do is reformat it so I can read and follow it easier.

Seems to me that this could be solved by a simple interface:

Code:

...
typedef std::vector<float> dimensionData;

class IShape
{
   public:
        virtual ~IShape() { }
        virtual float CalculateArea(const dimensionData &dimensions) = 0;
};

Each shape implementation would understand how to calculate it's own area. So long as the right number of parameters were passed in depending on the shape this would work. I used a vector b/c CalculateArea() does not conform to a common interface function since certain shapes require more or less info than others to calculate their area. But who knows better than the shape itself what parameters it will expect to see in the vector?. Ordering would also be important but so long as an agreement is made between the calling code and the CalculateArea() method of the class on the order of the parameters then this won't matter. This interpretation of the various vector elements would be hard-coded in the CalculateArea inside of the derived IShape class - but since this behavior is only inside of the IShape derived class I do not see a problem with it. I'm not sure what you are allowed to do in your assignment but you should always code to an interface.

Instead of writing a function for every type of shape you would write a class and a corresponding CalculateArea() that would know how to use the data in the vector being passed to it. All that is left is to create the shape based on input, push the data entered for the dimensions into the vector and pass the vector to IShape::CalculateArea().