Made a calculator please give advice

[ChrisMackle]

Hey, im new to C++ forums and C++ for that matter.
Im 16 years old and want to be a game developer so i made a calculator it took about 30min(a while i know but im a beginner) can someone tell me if what ive done is good code or is it really bad, i dont want to learn bad habits I decided to use a switch because i thought it would fit perfectly for such a basic calculator.

#include <iostream>

using namespace std;



int main ()
{
    int equation;
    int number1;
    int number2;
    int answer;
    cout << "Calculator v1" << endl;
    cout << "Choose an equation [1=add][2=subtract][3=divide][4=multiply]: ";
    cin >> equation;
    cout << "Enter first number: ";
    cin >> number1;
    cout << "Enter second number: ";
    cin >> number2;


    switch (equation){
      case 1://addition
        answer = number1 + number2;
        cout << number1 << " + " << number2 << " = " << answer << endl;
        break;
      case 2://subtraction
        answer = number1 - number2;
        cout << number1 << " - " << number2 << " = " << answer << endl;
        break;
      case 3://division
        answer = number1 / number2;
        cout << number1 << " / " << number2 << " = " << answer << endl;
        break;
      case 4://multiplication
        answer = number1 * number2;
        cout << number1 << " x " << number2 << " = " << answer << endl;
        break;
      default:
        equation, number1, number2, answer = 0;
        cout << "Math Error";
        }
    return 0;
}

Please post advice and let me know how i did,
Thanks.

[ChrisMackle]

anyone?

[WaltP]

anyone?

Don't be so anxious. This is a Forum of volunteers, not a 24-7 teaching site.

Your formatting looks very good. I'd add a few more comments explaining what's happening. And the comments you have IMO should be more like:

case 2:                   //subtraction

Next, change the program to to print the answer with one print instead of 4.

[foreshadowed]

You might also want to check that you're not dividing by zero before trying that operation.

[ChrisMackle]

Don't be so anxious. This is a Forum of volunteers, not a 24-7 teaching site.

Your formatting looks very good. I'd add a few more comments explaining what's happening. And the comments you have IMO should be more like:

case 2:                   //subtraction

Next, change the program to to print the answer with one print instead of 4.

What do you mean print the answer with one print instead of 4?

[WaltP]

You have the same print statement with only a minor change 4 times in your code. Try to figure out how to remove those 4 statements and add only 1 to print any of the 4 answers.

[rohan121212]

use floating point numbers instead of integers and use a loop because you have made a calculator that can calculate only once.
Please reply if you found these answers worthy

[ChrisMackle]

use floating point numbers instead of integers and use a loop because you have made a calculator that can calculate only once.
Please reply if you found these answers worthy

Ok I will do them 2 things tomorrow, I really want to finish this calculator because I tend to quit things so easily. Thanks for your reply :)

[xfbs]

If you are up to the challenge, you could also try to make the calculator figure out what operation it should perform. You could do something like this:

int lhs, rhs;  // right and left hand side
char operator; // operator
cin >> lhs;
cin >> operator;
cin >> rhs;

Then all you gotta do is figure out what operator it is ;)

Cheers, xfbs

[ChrisMackle]

If you are up to the challenge, you could also try to make the calculator figure out what operation it should perform. You could do something like this:

int lhs, rhs;  // right and left hand side
char operator; // operator
cin >> lhs;
cin >> operator;
cin >> rhs;

Then all you gotta do is figure out what operator it is ;)

Cheers, xfbs

What do you mean what operation it should perform?

[ChrisMackle]

You have the same print statement with only a minor change 4 times in your code. Try to figure out how to remove those 4 statements and add only 1 to print any of the 4 answers.

Ok ive done what you have said can you check the code i also did what the other person said which was to put it in a loop so you can do more than 1 calculation. Can you tell me if ive done anything wrong? it took couple of hours as i was changing alot of things around to make the code less confusing.
Heres my code now:

#include <iostream>
#include <string>

using namespace std;

//Calculator variables
string symbol;
double number1;
double number2;
double answer;
//Show math
void showMath(){
    cout << number1 << symbol << number2 << "=" << answer << endl;
}
//Quit
string quit = "N";
void restart(){
    cout << "Do you want to quit? (Y/N)";
    cin >> quit;
}


int main (){
    do{
        if (quit=="Y"||quit=="y"){
            return 0;
        }
        int equation;
        cout << "Calculator v1" << endl;
        cout << "Choose an equation [1=add][2=subtract][3=divide][4=multiply]: ";
        cin >> equation;
        cout << "Enter first number: ";
        cin >> number1;
        cout << "Enter second number: ";
        cin >> number2;


        switch (equation){
          case 1:                                           //addition
            answer = number1 + number2;
            symbol = "+";
            break;
          case 2:                                           //subtraction
            answer = number1 - number2;
            symbol = "-";
            break;
          case 3:                                           //division
            answer = number1 / number2;
            symbol = "/";
            break;
          case 4:                                           //multiplication
            answer = number1 * number2;
            symbol = "x";
            break;
          default:
            cout << "Math Error";
            return 0;
        }

        showMath();
        restart();
    }//close do
    while(quit=="N"||quit=="n");
}

[ChrisMackle]

use floating point numbers instead of integers and use a loop because you have made a calculator that can calculate only once.
Please reply if you found these answers worthy

I did what you said can you check it, i reposted my code on page 2

[WaltP]

My questions are:

1) Why the IF statement just after the DO?

do{
        if (quit=="Y"||quit=="y"){
            return 0;
        }
        . . . 
        restart();
    }//close do
    while(quit=="N"||quit=="n");

What does it do that the DO-WHILE doesn't do?

2) What happens if number2 is 0 and symbol is "/"?

3) Pass the values into the math function rather than use global values:

//Show math
void showMath(){
    cout << number1 << symbol << number2 << "=" << answer << endl;
}

Globals are generally considered a bad thing.

4) The restart function should return a value rather than use a global. Should it return a string or a boolean?

[ChrisMackle]

My questions are:

1) Why the IF statement just after the DO?

do{
        if (quit=="Y"||quit=="y"){
            return 0;
        }
        . . . 
        restart();
    }//close do
    while(quit=="N"||quit=="n");

What does it do that the DO-WHILE doesn't do?

Because the do-while only loops if quit==N so i added the if statement to also check for quit==Y otherwise it wont check for Y at all or is this not right? what else could i do?
2) What happens if number2 is 0 and symbol is "/"?
Dont really know what you mean, i tried it and it works fine.
3) Pass the values into the math function rather than use global values:

//Show math
void showMath(){
    cout << number1 << symbol << number2 << "=" << answer << endl;
}

Globals are generally considered a bad thing.
Can you help me out with this one, i dont know where else to put them because it doesnt work if they are in the showmath function.

4) The restart function should return a value rather than use a global. Should it return a string or a boolean?

String would be quicker otherwise i would need an if statement converting it to a boolean wouldnt i? what do you mean return a value rather than use a global, im totally new to this lol :P
Thanks heeps btw

[ChrisMackle]

My questions are:

1) Why the IF statement just after the DO?

do{
        if (quit=="Y"||quit=="y"){
            return 0;
        }
        . . . 
        restart();
    }//close do
    while(quit=="N"||quit=="n");

What does it do that the DO-WHILE doesn't do?

Because the do-while only loops if quit==N so i added the if statement to also check for quit==Y otherwise it wont check for Y at all or is this not right? what else could i do?

2) What happens if number2 is 0 and symbol is "/"?
It says inf, so i guess i need to write if (number2 < 1) { answer = 0; } thats right?

3) Pass the values into the math function rather than use global values:

//Show math
void showMath(){
    cout << number1 << symbol << number2 << "=" << answer << endl;
}

Globals are generally considered a bad thing.
Can you help me out with this one, i dont know where else to put them because it doesnt work if they are in the showmath function.

4) The restart function should return a value rather than use a global. Should it return a string or a boolean?
String would be quicker otherwise i would need an if statement converting it to a boolean wouldnt i? what do you mean return a value rather than use a global, im totally new to this lol :P
Thanks heeps btw

[timetraveller92]

small suggestion. just before "calculator v1" type do{
and b4 return 0; type cout<<"Do you want to try again (y/n)";cin>>retry;}while(retry=='y');
Also, declare char retry=='n'; at the start. Your code is perfect because u have used all the breaks properly. No break for default which is correct(it means u've understood how switch works) and yes, for all menu applications use switch! And you can also use the retry=getchar function from stdio.h library to get y/n instead of cin. that makes it seem more neat

[xfbs]

For example, when the user enters "34*56", then lhs will be 34, operator will be '*', and rhs will be 56. Then you could use a switch case thing to perform the right operation (when operator is '*' do multiplication, when it's '+' do addition, etc)

[WaltP]

My questions are:

1) Why the IF statement just after the DO?

do{
        if (quit=="Y"||quit=="y"){
            return 0;
        }
        . . . 
        restart();
    }//close do
    while(quit=="N"||quit=="n");

What does it do that the DO-WHILE doesn't do?

Because the do-while only loops if quit==N so i added the if statement to also check for quit==Y otherwise it wont check for Y at all or is this not right? what else could i do?

Why should it check for Y? You checked for N and continued. So you are assuming Y. Nothing wrong with that. Assuming your DO-WHILE test is correct. Is it?

2) What happens if number2 is 0 and symbol is "/"?
It says inf, so i guess i need to write if (number2 < 1) { answer = 0; } thats right?

Think deeper....

3) Pass the values into the math function rather than use global values:

//Show math
void showMath(){
    cout << number1 << symbol << number2 << "=" << answer << endl;
}

Globals are generally considered a bad thing.
Can you help me out with this one, i dont know where else to put them because it doesnt work if they are in the showmath function.

Look up functions and the complete syntax. Put the variables in main()

4) The restart function should return a value rather than use a global. Should it return a string or a boolean?
String would be quicker otherwise i would need an if statement converting it to a boolean wouldnt i? what do you mean return a value rather than use a global

You are using an IF anyway. It's implied in the DO-WHILE. And you are already putting a value into a global. Just move the variable into the function and return it instead.

[nuclear]

You could try to do more difficult calculations, you could create a function for it, make the user to enter the values required, send it to a function and make it return the answer.

[ChrisMackle]

For example, when the user enters "34*56", then lhs will be 34, operator will be '*', and rhs will be 56. Then you could use a switch case thing to perform the right operation (when operator is '*' do multiplication, when it's '+' do addition, etc)

That sounds tricky, il definitely try that after I've mastered this calculator :)

[creative_m]

your code looks good but try to make no repeated statments "print results.
also you can try to improve the way the user will interacte with your calc.i mean that the user can write "5+4" instead of 3 enters she/he would press.

[ChrisMackle]

Why should it check for Y? You checked for N and continued. So you are assuming Y. Nothing wrong with that. Assuming your DO-WHILE test is correct. Is it?

Im not sure, is it correct? can you tell me how else i could do this without putting the if quit==y inside the loop, because its kind of annoying having it inside the loop

2)Think deeper....
Ok so i put this in,

case 3:                                           //division
            answer = number1 / number2;
            symbol = "/";
            if (number2==0){
                cout << "Cannot divide by 0" << endl;
                answer = 0;
            }
            break;

3)Look up functions and the complete syntax. Put the variables in main() k il try that

4)You are using an IF anyway. It's implied in the DO-WHILE. And you are already putting a value into a global. Just move the variable into the function and return it instead.
I dont really get what you mean by this, can you give an example or post how it should be if you have time, it would really help.

[WaltP]

Im not sure, is it correct? can you tell me how else i could do this without putting the if quit==y inside the loop, because its kind of annoying having it inside the loop

What happened when you just erased the IF block as a test?

2)Think deeper....
Ok so i put this in,

case 3:                                           //division
            answer = number1 / number2;
            symbol = "/";
            if (number2==0){
                cout << "Cannot divide by 0" << endl;
                answer = 0;
            }
            break;

Let's see.
Do the divide.
Then test if you can't do the divide.
Something seems backwards to me.

3)Look up functions and the complete syntax. Put the variables in main() k il try that

Good

4)You are using an IF anyway. It's implied in the DO-WHILE. And you are already putting a value into a global. Just move the variable into the function and return it instead.
I dont really get what you mean by this, can you give an example or post how it should be if you have time, it would really help.

When you read up on functions you will understand.