How can I make this work and is there a way to optimize this?

Question

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;

    }

}

What is your ***specific*** question for Stackoverflow? Stackoverflow is not a tutorial site or a code review site, we only answer specific questions on programming topics. Before posting their first question on stackoverflow.com, everyone should take the [tour], read the [help], understand all the requirements for a [mre] and [ask] questions here. Not doing any of this results in a poor quality question almost every time. It then gets downvoted, closed, and then deleted. — Sam Varshavchik, Mar 21 '22 at 21:19
I know it's a bad practice to use a global variable but I couldn't get the second part of the switch to "see" the top1 variable. — Aydopower, Mar 21 '22 at 21:19
All the issues can be answered by Google. Part of learning to code is working through easy stuff like this on your own — sevensevens, Mar 21 '22 at 21:20
The downside of Google is until you know enough about a language to detect morons and frauds, you're gonna learn C++ from a lot of morons and frauds. You're a lot better off with [some good books](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) to get started. — user4581301, Mar 21 '22 at 21:26
BTW, the local variables you declare in the `case` statements will disappear after execution leaves the `case` statement. Try changing `int top1{` to `int top1 = {`. — Thomas Matthews, Mar 21 '22 at 22:06
Search the internet for "C++ shunting yard algorithm" to get a calculator that can handle operator precedence. — Thomas Matthews, Mar 21 '22 at 22:11

ChrisMM · Answer 1 · 2022-03-21T21:27:53.160

1

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.

edited Mar 21 '22 at 21:27

answered Mar 21 '22 at 21:24

ChrisMM

8,448
13
29
48

thank you for helping instead of scolding me like the others. – Aydopower Mar 21 '22 at 21:27
Another optimization is to extract the `switch` statements to a separate function. Thus the `main` will make two function calls to one set of statements. – Thomas Matthews Mar 21 '22 at 22:08

score 0 · Accepted Answer · answered Mar 21 '22 at 22:20

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);

How can I make this work and is there a way to optimize this?

2 Answers2