C++ Simple Calculator

Question

I'm very new to C++ and am trying to code simple programs to test my knowledge on the very basics of this language. Yesterday, I created a very simple calculator script which allows you to add, subtract, multiply, and divide and set of two numbers. While it is still very basic (e.g. only calculating two numbers), I plan on adding more functionality and improving the overall code-base to be not only more efficient, but a more pleasing experience to review. Remember, I am still VERY new to this and need some guidance (for example, I used goto(label) to escape nested loops, there's probably a much better way to do this) on how I can improve this code. Thanks.

main.cpp

#include <iostream>
#define log(x) std::cout << x;
#define logl(x) std::cout << x << std::endl;

int main() {
    int choice, choice2;
    float first, second;

    logl("-----------------------");
    logl("schmobbing's calculator");
    logl("-----------------------");

    start:
    std::cout << "" << std::endl;
    logl("Choose one of the following calculation options.");
    log("[1] Addition, [2] Subtraction, [3] Multiplication, [4] Division: ");
    std::cin >> choice;

    while (true) {
        switch (choice) {
            case 1:
                logl("");
                logl("You've chosen addition.");
                break;
            case 2:
                logl("");
                logl("You've chosen subtraction.");
                break;
            case 3:
                logl("");
                logl("You've chosen multiplication.");
                break;
            case 4:
                logl("");
                logl("You've chosen division.");
                break;
            default:
                logl("");
                if (!std::cin) {
                    std::cin.clear();
                    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
                    logl("You've chosen an invalid operator, please try again.");
                    log("[1] Addition, [2] Subtraction, [3] Multiplication, [4] Division: ");
                    std::cin >> choice;
                    continue;
                } else {
                    logl("You've chosen an invalid operator, please try again.");
                    log("[1] Addition, [2] Subtraction, [3] Multiplication, [4] Division: ");
                    std::cin >> choice;
                    continue;
                }
        }
        break;
    }

    while (true) {
        switch (choice) {
        case 1:
            while (true) {
                log("Enter your first number: ");
                std::cin >> first;
                if (!std::cin) {
                    std::cin.clear();
                    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
                    logl("");
                    logl("You've chosen an invalid number, please try again.");
                    continue;
                }
                break;
            }
            while (true) {
                log("Enter your second number: ");
                std::cin >> second;
                if (!std::cin) {
                    std::cin.clear();
                    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
                    logl("");
                    logl("You've chosen an invalid number, please try again.");
                    continue;
                }
                break;
            }
            logl("");
            std::cout << "The sum of the given numbers is " << first + second << "." << std::endl;
            while (true) {
                log("Would you like to calculate for a different result? [1] Yes or [2] No: ")
                    std::cin >> choice2;
                switch (choice2) {
                    case 1:
                        goto start;
                    case 2:
                        goto end;
                    default:
                        if (!std::cin) {
                            std::cin.clear();
                            std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
                            logl("");
                            logl("You've chosen an invalid option, please try again.");
                            break;
                        } else {
                            logl("");
                            logl("You've chosen an invalid option, please try again.");
                            break;
                        }
                }
            }
            break;
        case 2:
            while (true) {
                log("Enter your first number: ");
                std::cin >> first;
                if (!std::cin) {
                    std::cin.clear();
                    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
                    logl("");
                    logl("You've chosen an invalid number, please try again.");
                    continue;
                }
                break;
            }
            while (true) {
                log("Enter your second number: ");
                std::cin >> second;
                if (!std::cin) {
                    std::cin.clear();
                    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
                    logl("");
                    logl("You've chosen an invalid number, please try again.");
                    continue;
                }
                break;
            }
            logl("");
            std::cout << "The difference of the given numbers is " << first - second << "." << std::endl;
            while (true) {
                log("Would you like to calculate for a different result? [1] Yes or [2] No: ")
                    std::cin >> choice2;
                switch (choice2) {
                    case 1:
                        goto start;
                    case 2:
                        goto end;
                    default:
                        if (!std::cin) {
                            std::cin.clear();
                            std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
                            logl("");
                            logl("You've chosen an invalid option, please try again.");
                            break;
                        } else {
                            logl("");
                            logl("You've chosen an invalid option, please try again.");
                            break;
                        }
                }
            }
            break;
        case 3:
            while (true) {
                log("Enter your first number: ");
                std::cin >> first;
                if (!std::cin) {
                    std::cin.clear();
                    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
                    logl("");
                    logl("You've chosen an invalid number, please try again.");
                    continue;
                }
                break;
            }
            while (true) {
                log("Enter your second number: ");
                std::cin >> second;
                if (!std::cin) {
                    std::cin.clear();
                    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
                    logl("");
                    logl("You've chosen an invalid number, please try again.");
                    continue;
                }
                break;
            }
            logl("");
            std::cout << "The product of the given numbers is " << first * second << "." << std::endl;
            while (true) {
                log("Would you like to calculate for a different result? [1] Yes or [2] No: ")
                    std::cin >> choice2;
                switch (choice2) {
                    case 1:
                        goto start;
                    case 2:
                        goto end;
                    default:
                        if (!std::cin) {
                            std::cin.clear();
                            std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
                            logl("");
                            logl("You've chosen an invalid option, please try again.");
                            break;
                        } else {
                            logl("");
                            logl("You've chosen an invalid option, please try again.");
                            break;
                        }
                }
            }
            break;
        case 4:
            while (true) {
                log("Enter your first number: ");
                std::cin >> first;
                if (!std::cin) {
                    std::cin.clear();
                    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
                    logl("");
                    logl("You've chosen an invalid number, please try again.");
                    continue;
                }
                break;
            }
            while (true) {
                log("Enter your second number: ");
                std::cin >> second;
                if (!std::cin) {
                    std::cin.clear();
                    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
                    logl("");
                    logl("You've chosen an invalid number, please try again.");
                    continue;
                }
                break;
            }
            if (second == 0) {
                logl("");
                std::cout << "The quotient of the given numbers is undefined." << std::endl;
            } else {
                logl("");
                std::cout << "The quotient of the given numbers is " << first / second << "." << std::endl;
            }
            while (true) {
                log("Would you like to calculate for a different result? [1] Yes or [2] No: ")
                std::cin >> choice2;
                switch (choice2) {
                    case 1:
                        goto start;
                    case 2:
                        goto end;
                    default:
                        if (!std::cin) {
                            std::cin.clear();
                            std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
                            logl("");
                            logl("You've chosen an invalid option, please try again.");
                            break;
                        } else {
                            logl("");
                            logl("You've chosen an invalid option, please try again.");
                            break;
                        }
                }
            }
        }
        end:
        break;
    }
}
```

Answer 1

Welcome to C++!

---

Empty logl("")

I have seen a few them in your program. But I wasn't 100% sure as to why they keep appearing. My guess is that you are to print a newline. In that case, you should just use something called escape sequences. Specifically, '\n'

Printing '\n' Will just print a new line onto the terminal

std::cout << "Hel\nlo!";

// Output

Hel
lo!

Since you are new, another escape sequence that is often used is the \t. i.e the TAB. Using this lets you print 8 spaces onto the terminal.

std::cout << "Hel\tlo!";

// Output

Hel        lo!

---

'\n' vs std::endl

My previous point must've left you wondering whether you should use '\n' or std::endl. Since both of them will perform the task, which is to print a new line.

In this case, you should use '\n'. '\n' will be faster to run since std::endl calls std::flush. Basically, std::endl flushes the output stream every time you call it. Which isn't as fast as printing '\n'.

std::cout << std::endl;
std::cout << '\n' << std::flush; // equivalent to the top

"std::endl" vs "\n"

---

Formatting output

    logl("-----------------------");
    logl("schmobbing's calculator");
    logl("-----------------------");

    start:
    std::cout << "" << std::endl;
    logl("Choose one of the following calculation options.");
    log("[1] Addition, [2] Subtraction, [3] Multiplication, [4] Division: ");

How about you save this in a variable, and only print that? You can achieve this using the raw string literal.

auto Heading = R"(
        -----------------------
        schmobbing's calculator
        -----------------------
)";

auto Instructions = R"(
Choose one of the following calculation options.
[1] Addition, [2] Subtraction, [3] Multiplication, [4] Division
)";

auto InvaildOperator = R"(
You've chosen an invalid operator!
)"

std::cout << Heading;
std::cout << Instructions;

Here if you want to make tweaks to how the instructions would look, it would be very easy.

---

Repetition

                if (!std::cin) {
                    std::cin.clear();
                    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
                    logl("You've chosen an invalid operator, please try again.");
                    log("[1] Addition, [2] Subtraction, [3] Multiplication, [4] Division: ");
                    std::cin >> choice;
                    continue;
                } else {
                    logl("You've chosen an invalid operator, please try again.");
                    log("[1] Addition, [2] Subtraction, [3] Multiplication, [4] Division: ");
                    std::cin >> choice;
                    continue;
                }

You have written something twice here. Both the if statements do the exact same thing, except that the first one does something extra. Therefore, keep that extra in the if statement and move everything else out.

            if (!std::cin) {
                std::cin.clear();
                std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
            }
            logl("You've chosen an invalid operator, please try again.");
            log("[1] Addition, [2] Subtraction, [3] Multiplication, [4] Division: ");
            std::cin >> choice;
            continue;

+1 if you followed my previous advice.

            if (!std::cin) {
                std::cin.clear();
                std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
            }
            std::cout << InvalidOperator; 
            std::cout << Instructions;
            std::cin >> choice;
            continue;

---

Use a do-while loop

Instead of having goto which many programmers don't like, use a simple do-while loop.

char PlayAgain;

do {
 // ...
 // ... 


 // According to user's input, make `PlayAgain` y or n
} while(std::tolower(PlayAgain) == 'y');

---

Functions, please

The biggest thing missing in the program are functions. You have a huge bunch of code in your main() which is very hard to read. Split your work into different functions. Follow the Single-responsibility principle.

Kainev's answer gives a good example of how the program can look with proper structure.

---

No magic numbers

case 1:


case 2:


case 3:


case 4:

This was the switch statement from your program for the operator selections. Over here 1, 2, 3, and 4 don't necessarily mean anything. They're called magic numbers. i.e Unnamed numeric constants. A better thing to do would be to use an enum

enum Operator { Add = 1, Subtract, Multiply, Divide };


case Operator::Add:


case Operator::Subtract:

//...

Subtract, Multiply, and Divide are automatically set to 2, 3, and 4 respectively. Now you don't have meaningless numbers pollution your program.

Answer 2

Macros

It is important to be very careful when using macros. A major pitfall of C++ macros is that they ignore namespaces, for example:

#include <map>
#define map

int main()
{
    std::map<int, int> my_map;
}

The macro map will completely ignore the std namespace, this results in std::map_macro_inserted_here<int, int> my_map which will cause a compiler error.

This is why it is highly recommended to name all macros in UPPER_CASE, this lets you know its a macro and helps avoid naming clashes.

I don't mind the use of a LOG macro, however it is more often used as a convenience for a more complicated logging system, rather than a single line cout.

while(true)

This isn't necessarily bad, and can be useful, however if there is a logical condition that can be used instead of true, you should preference that. It makes the intent clearer, and decreases the chance of getting stuck in the loop due to a logical error. To see this in action, look at the menu_selection function in my example code.

goto and code reuse

In general if you ever feel the need to write a goto statement, you should probably be writing a function instead.

When reading through your code, if you start to find patterns where you've written the same or even similar code in multiple places, this is also a sign you should be writing a function instead.

There are a couple of benefits for doing this including:

• clearer/more concise code, making it easier for others to read.
• Better maintainability

Example

Here is a possible refactor. It is 87 lines long, with whitespace for readability, versus the original 272 with almost no whitespace. It's definitely not perfect, but hopefully demonstrates the idea of code reuse and the benefits it has:

Update: Added enum's to eliminate "magic" numbers as suggested in Aryan's answer.

#include <iostream>

enum Operator { Add = 1, Subtract = 2, Multiply = 3, Divide = 4 };
enum ExitMenuOption { Continue = 1, Exit = 2 };

int menu_selection(const std::string& menu, int min_option, int max_option);
float get_float_input(const std::string& prompt);
void calculator(Operator operation);

int main()
{
    const std::string title_bar = "-----------------------\nschmobbing's calculator\n-----------------------";
    const std::string operation_menu = "Choose one of the following calculation options.\n[1] Addition, [2] Subtraction, [3] Multiplication, [4] Division: ";
    const std::string exit_menu = "Would you like to calculate for a different result? [1] Yes or [2] No: ";

    std::cout << title_bar << "\n";

    do
    {
        Operator operation = static_cast<Operator>(menu_selection(operation_menu, 1, 4));
        calculator(operation);
    } while (menu_selection(exit_menu, 1, 2) != ExitMenuOption::Exit);
}

int menu_selection(const std::string& menu, int min_option, int max_option)
{
    int choice = -1;

    while (choice < min_option || choice > max_option)
    {
        std::cout << menu;
        std::cin >> choice;

        if (!std::cin)
        {
            std::cin.clear();
            std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
            std::cout << "\nYou've chosen an invalid option, please try again.\n";
        }
    }

    std::cout << "\n";
    return choice;
}

float get_float_input(const std::string& prompt)
{
    float input;

    do
    {
        std::cout << prompt;
        std::cin >> input;

        if (std::cin)
        {
            break;
        }
        else
        {
            std::cin.clear();
            std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
            std::cout << "\nYou've chosen an invalid number, please try again.";
        }
    } while (true);

    return input;
}

void calculator(Operator operation)
{
    float first = get_float_input("Enter your first number: ");
    float second = get_float_input("Enter your second number: ");

    switch (operation)
    {
        case Operator::Add:
            std::cout << "\nThe sum of the given numbers is " << first + second << ".\n";
            break;
        case Operator::Subtract:
            std::cout << "\nThe difference of the given numbers is " << first - second << ".\n";
            break;
        case Operator::Multiply:
            std::cout << "\nThe product of the given numbers is " << first * second << ".\n";
            break;
        case Operator::Divide:
            if (second == 0)
                std::cout << "\nThe quotient of the given numbers is undefined.\n";
            else
                std::cout << "\nThe quotient of the given numbers is " << first / second << ".\n";
            break;
    }
}