advice on this C++ script I wrote (simple calculater program but my first!)
Author
Could some one please give me advice on how to make this script better? I will be adding more features to it but I would like to know what I can do to make the script I already have look better or anything! Any possitive (another words helpful and not rude) advice on it is greatly needed thanks!
#include <cstdlib>
#include <iostream>
using namespace std;
int a;
int b;
// Addition code
int addition (int a, int b)
{
int r;
r=a+b;
return (r);
}
int subtract (int a, int b)
{
int r;
r=a-b;
return (r);
}
int multi (int a, int b)
{
int r;
r=a*b;
return (r);
}
int main(int argc, char *argv[])
{
int (c);
cout << "Hello \n";
cout << "My name is James \nI will be your calculator today! \n";
do {
cout << "\n\nplease tell me what you would like to do: \n1= Addition\n2= Subtraction\n3=multiply\n\n";
cin >> c;
if (c == 1)
{int z;
cout << "please enter your first number";
cin >> a;
cout << "please enter your secound number";
cin >> b;
z=addition (a, b);
cout << "The result is " << z;
cout << "\n";}
else if (c == 2)
{int z;
cout << "please enter your first number";
cin >> a;
cout << "please enter your secound number";
cin >> b;
z=subtract (a, b);
cout << "The result is " << z;
cout << "\n";}
else if (c == 3)
{int z;
cout << "please enter your first number";
cin >> a;
cout << "please enter your secound number";
cin >> b;
z=multi (a, b);
cout << "The result is " << z;
cout << "\n";}
cout << "would you like to continue?\nPress:\n1 for yes\n2 for no\n";
cin >> c;
} while (c != 0);
system("PAUSE");
}
Maybe making int r a global along with a and b? I guess that wouldn't make it better, but it would save the repeated calls, and might speed up compile time, and run speed.... Looks pretty good from my view, but I'm by no means an expert.
thats alot of code for a simple calculator. I would code main() like this:
Note: I didnt compile this so there may be afew minor errors.
Basically all I did was move the "please enter numbers" part, and the part that prints the result, outside of the if statements. that cut down on redundant code. Congrats on your first program by the way.
Edit: fixed afew things
int main(){ int c; int z; cout<<"Hello \n My name is James \nI will be your calculator today! \n"; do{ cout<<"\n\nplease tell me what you would like to do: \n1= Addition\n2= Subtraction\n3=multiply\n\n" cin>>c; cout<<"Please enter the first number"; cin>>a; cout>>"Please enter the second number"; cin>>b; if(c==1) z=addition(a,b); if(c==2) z=subtraction(a,b); if(c==3) z=multi(a,b); cout<<"the result is: "<<z<<"\n"; } while(c); system("pause"); return 0;}Note: I didnt compile this so there may be afew minor errors.
Basically all I did was move the "please enter numbers" part, and the part that prints the result, outside of the if statements. that cut down on redundant code. Congrats on your first program by the way.
Edit: fixed afew things
I would suggest writing an RPN calculator. Then you could write a calculator that would returns results for whole expressions for all math operators and such, very easy to implement too.
Good luck!
Good luck!
Im still a beginner but i would put:
if (c != 1)
break;
before you end your do loop to make the loop actually end, because when the user enters 2 at the end to end the calculator, it will start the loop again.
Also, take out the:
while (c != 0);
after the loop to end it. Another thing, make the "do" loop a "while" by erasing "do" and putting "while (true)" instead. Hopefully you understand what im saying but that it. Good luck :).
-Pharaoh12
if (c != 1)
break;
before you end your do loop to make the loop actually end, because when the user enters 2 at the end to end the calculator, it will start the loop again.
Also, take out the:
while (c != 0);
after the loop to end it. Another thing, make the "do" loop a "while" by erasing "do" and putting "while (true)" instead. Hopefully you understand what im saying but that it. Good luck :).
-Pharaoh12
Maybe you should use a switch statement instead of repeaded if statements? It might make your code more readible.
You may have noticed that you have the code
... Honest to God I can not figure out a way to insert tabs on this Unix machine.
int z;cout << "please enter your first number";cin >> a;cout << "please enter your secound number";cin >> b;do {cout << "\n\nplease tell me what you would like to do: \n1= Addition\n2= Subtraction\n3=multiply\n\n";cin >> c;int z;cout << "please enter your first number";cin >> a;cout << "please enter your secound number";cin >> b;if (c == 1) z = addition (a, b);else if (c == 2) z = subtract (a, b);else if (c == 3) z = multi (a, b);cout << "The result is " << z;cout << "\n";}... Honest to God I can not figure out a way to insert tabs on this Unix machine.
Very nice!
What I would have done is, to use different functions... Let me explain:
Additions, subtraction, multiplication, etc. is mostly built-in... So instead of:
I would just use:
It saves the extra variables and function call...
Instead, I would make a function to handle input and some constants:
...Or something along the lines of that...
Good job and good luck!
What I would have done is, to use different functions... Let me explain:
Additions, subtraction, multiplication, etc. is mostly built-in... So instead of:
int result = subtraction(2, 1);I would just use:
int result = 2 - 1;It saves the extra variables and function call...
Instead, I would make a function to handle input and some constants:
// Better readabilityconst int USER_ADD = 0, USER_MULTIPLY = 1, USER_DIVIDE = 2, USER_SUBTRACT = 3;// Pretty much it logicalvoid HandleInput(const int Input) { // We won't be modifing the parameter switch (Input) { // A switch state instead of repeated if/else case USER_ADD: // Add break; case USER_MULTIPLY: // Multiply break; // Others following the same rule}int main() { while (true) { cout << "0 - Add\n1 - Mult.\n2 - Div.\n3 - Sub.\n4 - Exit\n\n"; int Choice; cin >> Choice; if (Choice == 4) // Exit break; HandleInput(Choice); }}...Or something along the lines of that...
Good job and good luck!
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
