First successful project

[ticonwc3]

Hello, I am new to C++ and I have made a calculator that adds, subtracts, divides and multiplies. Would you please recommend some changes i could make to the code?
please do not gripe about the system commands however, i know that they are bad because of security but regardless this is just to better myself

#include <iostream>

using namespace std;
//allows the selecting of two numbers which will be added together'
//variables a and b and c are global
int a;
int b;
int c;
//adding
int add() 
{
    cout << "Please enter the two numbers to add together\n";
    cin >> a;
    cin >> b;
    c = a + b;
    cout << "The sum is " << c << "\n";
}
//subtracting
int subtract()
{
    cout << "Please enter two numbers\n";
    cin >> a;
    cin >> b;
    c = a - b;
    cout << "The result is " << c << "\n";
}
//multiplying
int multiply()
{
    cout << "Please enter two numbers to be multiplied\n";
    cin >> a;
    cin >> b;
    c = a * b; 
    cout << "The result is " << c << "\n";
}
//dividing
int divide()
{
    cout << "Please enter two numbers\n";
    cin >> a;
    cin >> b;
    c = a / b;
    cout << "The result is " << c << "\n";
}
//main function
int main()
{
int menunumber;

    cout << "Welcome to Ryan's calculator, What would you like to do?\nPlease choose between\n 1 to |add|\n 2 to |subtract|\n 3 to |multiply|\n 4 to |divide|\n";
    cin >> menunumber;
    if (menunumber == 1)
    {
                  add();
}

    if (menunumber == 2)
    {
    subtract();
}
    if (menunumber == 3)
    {
                  multiply();
                  }
    if (menunumber == 4)
    {
                  divide();
                  }
    else if (menunumber <= 0)
    {
        cout << "Invalid selection";
        system("pause");
        }
    else if (menunumber > 4)
    {
         cout << "Invalid Selection\n";
         system("pause");
         }
         system("pause");
         system("cls");
        main();
        }

[jsmith]

Look at your four functions -- the first three lines of each are identical, and
the last one is identical. Consider refactoring the code such that you write
the prompt and get the input in only one place in the program instead of four.

[Lv0f92yv]

Also, your functions are returning an 'int', but do not actually return anything. You're probably getting compile time warnings/errors about this?

Maybe you could rewrite the functions to take in the two numbers to perform the operation with as parameters, and return the result.

Edit: Also, the call to main() at the end of your program is recursive - which in and of itself is fine - but there are better ways to accomplish the same task, as mentioned above - consider looping.

[ticonwc3]

By returning something you mean the line return 0; right?

[Vexer]

I would define your functions(except for Main) as a void. Also, I would use a switch() statement to cycle through the different menunumbers.

[Disch]

Also, the call to main() at the end of your program is recursive

Not only is it recursive, it's also illegal.

You're not supposed to call main ever, from anywhere in your program.

[Lv0f92yv]

By returning something you mean the line return 0; right?

If that is always the value you want to return to the caller. You can return any value so long as its type matches that of what the function is declared to return.

Small example:

int add(a, b)
{
   return a + b;
}

where a + b is the add operation on the parameters, a and b passed in.

[Albatross]

Disch wrote:

Not only is it recursive, it's also illegal.

So we're now calling "evil" features of C++ "illegal"? :P

Your main function has no return, just like the rest of your functions. This can't possibly compile. Sorry.

-Albatross

[Lv0f92yv]

Indeed... I didn't want to call it out since I'm not that experienced, but I was sure the above would never compile, ever, no matter what, ever ever ever. Never.

[packetpirate]

Please explain how you expect to "better yourself" by writing shitty code.

[Lv0f92yv]

I think OP's just trying to get some practice with the basics.

[chrisname]

@packetpirate,
You go too far. I'm sure you were a beginner once.

[blackcoder41]

+1 chrisname

[z05DSL3A]

packetpirate wrote:

Please explain how you expect to "better yourself" by writing shitty code.

You write code, post it on a forum and helpful people give you advice on how to improve it, or some numb nuts tells you its shitty.

[Galik]

cout << "Welcome to Ryan's calculator, What would you like to do?\nPlease choose between\n 1 to |add|\n 2 to |subtract|\n 3 to |multiply|\n 4 to |divide|\n";

Long constants strings like that can be broken into smaller pieces like this:

const char* str = "some text" " broken into" " several pieces.";

Notice that there are no symbols between the separate quoted pieces of text?

In that case the compiler will simply concatenate them all together into one long string.

This means that the following two strings are identical:

const char* str1 = "some text" " broken into" " several pieces.";
const char* str2 = "some text broken into several pieces.";

The benefit of this is that you can define your string constants on multiple lines by breaking them into sizable chunks:

const char* str1 =
    "some text"
    " broken into"
    " several pieces.";

This helps to make very long lines much more readable.

Like your line for instance could be like this:

cout << "Welcome to Ryan's calculator, "
    "What would you like to do?\n"
    "Please choose between\n"
    " 1 to |add|\n"
    " 2 to |subtract|\n"
    " 3 to |multiply|\n"
    " 4 to |divide|\n";

[ticonwc3]

@packetpirate
go back under your bridge. To everyone else, thank you for the info you actually contributed.

[moorecm]

Additionally, the global variables are never used outside of the functions; they have no business being global at all. Global variables are something best avoided whenever possible.