'How can I make this work and is there a way to optimize this? [closed]
I wanted to do a calculator with c++, and I did not wish my calculator to do only 2 number calculations, so I "made" 2 operator calculator. Also, I want to learn if there is a way to make this without two switch statements to make it easier for machines and programmers to read.
#include <iostream>
int top1 ;
int main()
{
char operatr1, operatr2;
float num1, num2, num3, num4 ;
std::cout << "Please choose the first operator ( +, -, *, /):";
std::cin >> operatr1 ;
std::cout << "Please choose the second operator ( +, -, *, /):";
std::cin >> operatr2;
std::cout << "Please enter three numbers: " << std::endl;
std::cin >> num1 >> num2 >> num3;
switch (operatr1) {
case '+':
int top1{ num1 + num2 };
break;
case '-':
int top1{ num1 - num2 };
break;
case '*':
int top1{ num1 * num2 };
break;
case '/':
int top1{ num1 / num2 };
break;
default:
std::cout << "Error! The operator is not correct" << std::endl << "The operators are ( +, -, *, / ).";
break;
}
switch (operatr2) {
case '+':
int top2{ top1 + num3 };
std::cout << "The answer is:" << " " << top2;
break;
case '-':
int top2{ top1 - num3 };
std::cout << "The answer is:" << " " << top2;
break;
case '*':
int top2{ top1 * num3 };
std::cout << "The answer is:" << " " << top2;
break;
case '/':
int top2{ top1 / num3 };
std::cout << "The answer is:" << " " << top2;
break;
default:
std::cout << "Error! The operator is not correct" << std::endl << "The operators are ( +, -, *, / ).";
break;
}
}
Solution 1:[1]
You could make use of a function:
int Evaluate(const char operator,
const int num1,
const int num2)
{
bool is_valid_operator = true;
int result = 0;
switch (operator)
{
case '+':
result = num1 + num2;
break;
case '-':
result = num1 - num2;
break;
case '/':
// Note: integer division.
result = num1 / num2;
break;
case '*':
result = num1 * num2;
break;
default:
result = 0;
is_valid_operator = false;
break;
}
if (is_valid_operator)
{
std::cout << "Result of " << num1 << " " << operator << " " << num2 << "\n";
std::cout << result << "\n";
}
return result;
}
Your main would be simplified to:
//...
int top1 = Evaluate(operatr1, num1, num2);
int top2 = Evaluate(operatr2, top1, num3);
Solution 2:[2]
Define top1 outside the switch statement. The global top1 you define doesn't have the value you expect, because you define a new top1 inside each case statement. Your compiler should be warning you about this (actually, I think this should be an error in this case).
Further, since num1 through num4 are floats, your top1 and top2 should really also be floats, not ints
Effectively, remove int top1 from your global variables, then in your main:
float top1 = 0;
switch ( operatr1 ) {
case '+':
top1 = num1 + num2;
break;
// ... continue with other cases here
}
Then, for your second switch statement, top1 will be visible and have the correct value.
Sources
This article follows the attribution requirements of Stack Overflow and is licensed under CC BY-SA 3.0.
Source: Stack Overflow
| Solution | Source |
|---|---|
| Solution 1 | Thomas Matthews |
| Solution 2 |
