Thanks

[3wpDGNh0]

please delete this topic

[TheIdeasMan]

Hi,

First up, please edit your post so it has code tags, how to use them in this article:

http://www.cplusplus.com/articles/z13hAqkS/

One can also use the format menu on the right, the <> button does code tags.

OK, a bunch of things I would change:

Variable names: Anyone should be able to tell straight away what a variable is for. E.g. what is lowerbase ? r might be ok for radius but I would prefer Radius

In C++, one can delay the declaration of a variable until it is needed, generally right up until the point where one has a sensible value to assign to it, unless it's inside a loop. This helps avoid problems from not initialising variables, and make s the code easier to read.

I make some suggestions on the names and types here:

int selection; // MenuSelection
double lowerbase;  // ? Deduced for the trapezoid from reading the code
double upperbase;  // ? But I don't want to deduce anything
double TrapezoidArea;
double TrapezoidHeight;
double TrapezoidParallelSide1;
double TrapezoidParallelSide2;
double Radius;               
double CircleArea;        
double RectangleLength;       
double RectangleWidth;          
double RectangleArea;  
double TriangleBase;
double TriangleHeight;
double TriangleArea;

const double PI = 3.14159;  // Use 3.14159 for π 

You should make more use of functions. The Menu could be in a void function; each case of the switch should call it's own function.

With this:
if (selection <=0 || selection > 5) //User choice validation

There isn't a need for it, the default: case of the switch handles it.

You could put the whole switch into a bool controlled while loop that incorporates the menu:

bool Quit = false;

while (!Quit) {
    Menu();
    std::cin >> MenuSelection;

    switch(MenuSelection ) {
        // cases
        case: 1
            ProcessCircle();

        case 5:
             Quit = true;
             std::cout << "Quitting Menu";  
             break;       

        default:
              // Display error mesg
    }
}

You have the following logic for different variables, consider having a bool IsPositiveNumber(const double Number) function instead.

if (r < 0)
{
cout << " enter a valid number.\n";
cin >> r;
}

With this:
circle = 3.14159 * pow(r, 2);

Use your PI variable, and it's more efficient to multiply rather than use the pow function to square something:

CircleArea = PI * Radius * Radius;

With functions in general, it's a good idea to have them do a calculation and return a result which can be printed later, rather than printing inside the function. One can do a function call in a std::cout statement.

With:
triangle = base * height * .5;

I always put numbers before and after the decimal point, it's clearer:

TriangleArea = TriangleBase * TriangleHeight * 0.5;

Hope this helps :+)