This is a nice first project. Since you are a beginner in C++, I'm not going to worry about the using namespace std
and a couple other details.
Naming
Your functions follow an inconsistent naming style and are not spelled correctly:
int pluss(int num1, int num2);
int Minus(int num1, int num2);
int devide(double num1, double num2);
int multiply(double num1, double num2);
These would be better named
int plus(int num1, int num2);
int minus(int num1, int num2);
int divide(double num1, double num2);
int multiply(double num1, double num2);
Consider using void functions
Your functions all return an int
. It's not very clear what the return value would mean, so I was puzzled over that for a bit, but then I noticed that they all returned 0
:
int pluss(int num1, int num2)
{
...
return 0;
}
It seems that you are copying the idea of int main()
, where main
returns an error code if something went wrong, but there's no way for something to go wrong in these functions barring an exception, so just make these functions void:
void pluss(int num1, int num2)
{
int answer = num1 + num2;
cout << num1 << " + " << num2 << " = " << answer << endl;
system("pause");
system("CLS");
}
On the other hand, the names of your functions seem to suggest that they should return the result of the operation, in which case you might do this:
int plus(int num1, int num2)
{
return num1 + num2;
}
But don't do this. This is simply re-inventing the +
operator.
System dependency
Your code uses system()
all over the place. system
is not cross platform, and it is better to make the code as cross-platform as possible. You really don't need to use system("CLS")
everywhere; people using a console don't expect their console to clear old messages. Furthermore, system("pause")
isn't necessary; the console waits whenever your program needs to obtain input.
Switch
In your main function, you have this:
if (begin == 1) {
...
}
else if (begin == 2) {
...
}
else if (begin == 3) {
...
}
else if (begin == 4) {
...
}
else {
...
}
This is basically a switch
statement; you could just code that like so:
switch (begin) {
case 1: ...
case 2: ...
case 3: ...
case 4: ...
default: ...
}
Why only add and subtract ints?
If you think it's a good idea to be able to divide and multiply doubles, why not have the same thing for add and subtract? That's definitely something I would expect if I were using your calculator.
Functions should only do "one" thing
Your functions do multiple things:
void multiply(double num1, double num2)
{
double answer = num1 * num2;
cout << num1 << " * " << num2 << " = " << answer << endl;
system("pause");
system("CLS");
}
Notice how this function is less of a mathematical function, and more of a "perform this task". The task you are having it perform is to "display the result of multiplication, then clear the screen afterwards". That's most definitely multiple things. It would be better to make the function this:
void showMultiply(double num1, double num2)
{
double answer = num1 * num2;
cout << num1 << " * " << num2 << " = " << answer << endl;
}
But if you were to do this with all your functions, suddenly they are all almost the same:
void showPlus(double num1, double num2)
{
double answer = num1 + num2;
cout << num1 << " + " << num2 << " = " << answer << endl;
}
void showMinus(double num1, double num2)
{
double answer = num1 - num2;
cout << num1 << " - " << num2 << " = " << answer << endl;
}
void showMultiply(double num1, double num2)
{
double answer = num1 * num2;
cout << num1 << " * " << num2 << " = " << answer << endl;
}
void showDivide(double num1, double num2)
{
double answer = num1 / num2;
cout << num1 << " / " << num2 << " = " << answer << endl;
}
We can collapse these down to result in less duplication:
void showCalculation(double num1, double num2, double answer, char operator)
{
cout << num1 << ' ' << operator << ' ' << num2 << " = " << answer << endl;
}
Now when we use it, it would look like this:
double answer = num1 + num2;
showCalculation(num1, num2, answer, '+');
...
However, since this function is only one line of code, I personally wouldn't use the function.
Code duplication
Notice how each block in the if statements look almost identical:
if (begin == 1) {
system("CLS");
int x;
int y;
cout << "Give me a first number" << endl;
cin >> x;
system("CLS");
cout << "Give me a second number" << endl;
cin >> y;
pluss(x, y);
return main();
}
else if (begin == 2) {
system("CLS");
int x;
int y;
cout << "Give me a first number" << endl;
cin >> x;
system("CLS");
cout << "Give me a second number" << endl;
cin >> y;
Minus(x, y);
return main();
}
else if (begin == 3) {
system("CLS");
double x;
double y;
cout << "Give me a first number" << endl;
cin >> x;
system("CLS");
cout << "Give me a second number" << endl;
cin >> y;
devide(x, y);
return main();
}
else if (begin == 4) {
system("CLS");
double x;
double y;
cout << "Give me a first number" << endl;
cin >> x;
system("CLS");
cout << "Give me a second number" << endl;
cin >> y;
multiply(x, y);
return main();
}
This is a sign that you should write functions to break it up. This is what I'd do:
double promptForInput(const string &message)
{
double result;
cout << message << endl;
cin >> result;
return result;
}
...
if (begin == 1) {
double num1 = promptForInput("Give me the first number");
double num2 = promptForInput("Give me the second number");
double result = num1 + num2;
showCalculation(num1, num2, result, '+');
}
// and so on
Calling main() is undefined behaviour
It is not legal to call main()
in your C++ code. You are using this to loop; you should instead learn how to use loops. This is one example of how to do so:
int main()
{
do {
int begin;
cout << "----------------------------------------" << endl;
cout << "-----Welcome to C++ Based Calculator!---" << endl;
cout << "-1=Plus--2=Minus--3=devide--4=multiply--" << endl;
cout << "----------------------------------------" << endl;
cin >> begin;
if (begin == 1) {
...
}
...
else {
break;
}
} while (true);
return 0;
}
Use constants
Since you are using numbers to represent what choice the user made, I strongly recommend you store them as constants:
constexpr int PLUS = 1;
constexpr int MINUS = 2;
constexpr int DIVIDE = 3;
constexpr int MULTIPLY = 4;
Final result
After apply all my recommendations, this is my final result:
#include <string>
#include <iostream>
using namespace std;
double promptForInput(const string &message)
{
double result;
cout << message << endl;
cin >> result;
return result;
}
constexpr int PLUS = 1;
constexpr int MINUS = 2;
constexpr int DIVIDE = 3;
constexpr int MULTIPLY = 4;
int main()
{
do {
int choice; // choice is a better name for this
cout << "----------------------------------------" << endl;
cout << "-----Welcome to C++ Based Calculator!---" << endl;
cout << "-1=Plus--2=Minus--3=Divide--4=Multiply--" << endl;
cout << "----------------------------------------" << endl;
cin >> choice;
double num1 = promptForInput("Give me the first number");
double num2 = promptForInput("Give me the second number");
double result;
char operatorName;
switch (choice) {
case PLUS:
result = num1 + num2;
operatorName = '+';
break;
case MINUS:
result = num1 - num2;
operatorName = '-';
break;
case DIVIDE:
result = num1 / num2;
operatorName = '/';
break;
case MULTIPLY:
result = num1 * num2;
operatorName = '*';
break;
default:
cout << "Some message thanking the user for using the calculator (optional). " << endl;
return 0; // to exit main
}
cout << num1 << ' ' << operatorName << ' ' << num2 << " = " << result << endl;
} while (true);
}
Going further
If you wanted to make this "even better", this is how I'd fully implement this:
#include <iostream>
#include <sstream>
#include <string>
void printOptions() {
std::cout << "------------------------------------------" << std::endl;
std::cout << "----- Welcome to C++ Based Calculator! ---" << std::endl;
std::cout << "- Plus: + ---" << std::endl;
std::cout << "- Minus: - ---" << std::endl;
std::cout << "- Divide: / ---" << std::endl;
std::cout << "- Multiply: * ---" << std::endl;
std::cout << "- Quit: Enter ---" << std::endl;
std::cout << "------------------------------------------" << std::endl;
}
constexpr char QUIT = '\0';
char menu() {
printOptions();
std::string input;
std::getline(std::cin, input);
if (input.size() != 1) {
return QUIT;
}
return input[0];
}
double promptForInput(const std::string &message) {
std::cout << message << std::endl;
double result;
std::istringstream input;
do {
std::string line;
std::getline(std::cin, line);
input.str(line);
input >> result;
if (!input) {
std::cout << "Please input a valid number" << std::endl;
}
} while (!input);
return result;
}
double performOperation(double num1, double num2, char choice) {
switch (choice) {
case '+': return num1 + num2;
case '-': return num1 - num2;
case '*': return num1 * num2;
case '/': return num1 / num2;
}
std::cerr << "Internal Error" << std::endl;
std::exit(1);
}
int main() {
do {
char choice = menu();
if (choice == QUIT) return 0; // The user wants to quit
double num1 = promptForInput("Give me the first number");
double num2 = promptForInput("Give me the second number");
double result = performOperation(num1, num2, choice);
std::cout << num1 << ' ' << choice << ' ' << num2 << " = " << result << std::endl;
} while (true);
}
Here's what I did differently:
- I had the user input the actual character for the operation they want to use. This makes my life a little easier, as I can just pass that around. If I didn't want to do this, I'd actually translate the number into the character.
- I didn't use the
using namespace std;
. That statement pollutes your namespace with all the symbols from the standard namespace; there's a reason why the standard namespace is only 3 characters long
- I used
std::getline
to get the input. This makes it so that the user has to input everything on its own line. I liked this idea better.
- I moved the
switch
into its own function; this makes the main function shorter, making it easier to understand.