Basic Calculator

[Bogeyman]

Hello. I have recently made a basic calculator using the operators (+, -, *, /).
I am looking for any suggestions or advice on how to improve/expand it and to help me with numerical input error handling. Thank you.

#include "std_lib_facilities.h" // Header file made by Bjarne Stroustrup.

int main()
{
    double no1 = 0, no2 = 0, result = 0;
    char myoperator = 0;
    cout << "This is a basic calculator. \n" << endl;

    start:
    cout << "Please enter a number: ";
    cin >> no1;
    /* Here is where I would like to put some code to handle any incorrect input.
    For example, if the user inputs a character or symbol instead of a number,
    I would like the program to output an error message and return to start. */
    cout << "Please choose an operator (+, -, *, /): ";
    cin >> myoperator;

    if (myoperator == '+')
    {
        cout << "The operator you have chosen is '+'.\nPlease enter a second number: ";
        cin >> no2; // Same as for int no1.
        result = no1 + no2;
        cout << no1 << " " << '+' << " " << no2
        << " " << '=' << " " << result << "\n\n";
        goto start;
    }
    else if (myoperator == '-')
    {
        cout << "The operator you have chosen is '-'.\nPlease enter a second number: ";
        cin >> no2;
        result = no1 - no2;
        cout << no1 << " " << '-' << " " << no2
        << " " << '=' << " " << result << "\n\n";
        goto start;
    }
    else if (myoperator == '*')
    {
        cout << "The operator you have chosen is '*'.\nPlease enter a second number: ";
        cin >> no2;
        result = no1 * no2;
        cout << no1 << " " << '*' << " " << no2
        << " " << '=' << " " << result << "\n\n";
        goto start;
    }
    else if (myoperator == '/')
    {
        cout << "The operator you have chosen is '/'.\nPlease enter a second number: ";
        cin >> no2;
        result = no1 / no2;
        cout << no1 << " " << '/' << " " << no2
        << " " << '=' << " " << result << "\n\n";
        goto start;
    }
    else
    {
        cout << "That operator has not been defined. Please try again.\n\n";
        goto start;
    }

    return 0;
}

[Chervil]

One improvement would be to reduce the amount of duplicated code, you have four blocks of code which are virtually identical (or could be made identical) apart from just one line.

Another improvement would be to replace the goto with a structured loop, such as a while loop. Although perfectly legible here, as programs become larger and more complex, the goto can result in unstructured, chaotic code, which is difficult to understand and maintain.

http://www.cplusplus.com/doc/tutorial/control/

[AceDawg45]

Well, as you may or may not know, computers / calculators cannot divide by zero. If you try to do it, the program will crash. So, you could handle that error with an if statement seeing if no2 is 0. If it is, just have a message show up or something.

[Bogeyman]

Thanks for the replies.

@Chervil;
How would I reduce the amount of duplicated code? As far as I know, I need to structure the code using if and else blocks to get it to work properly. Thanks for the advice on goto. How might I incorporate a while loop into my code in place of goto?

@AceDawg45;
You mean like this?

else if (myoperator == '/')
    {
        dividestart:
        cout << "The operator you have chosen is '/'.\nPlease enter a second number: ";
        cin >> no2;
        if(no2 == 0)
        {
            cout << "Cannot divide by 0! \n";
            goto dividestart;
        }
        result = no1 / no2;
        cout << no1 << " " << '/' << " " << no2
        << " " << '=' << " " << result << "\n\n";
        goto start;
    }

What I meant by numerical input error handling was to have the program output an error message should the user input something other than a numeric value when prompted for a numeric value.

So basically, what I am trying to do is this;

if(not a number)
{
  // Error message. Back to input line.
}

I would appreciate if someone could show me how to do that.

[Chervil]

First, here's a way to reduce some of the code duplication:

#include <iostream>

using namespace std;

int main()
{
    double no1 = 0, no2 = 0, result = 0;
    char myoperator = ' ';
    
    while (true)
    {
        cout << "This is a basic calculator. \n\n"
                "Please enter a number: ";
                
        cin >> no1;
        
        cout << "Please choose an operator (+, -, *, /): ";
        cin >> myoperator;
        
        if (myoperator == '+' || myoperator == '-' || myoperator == '*' || myoperator == '/')
        {
            cout << "The operator you have chosen is "
                 << myoperator << "\nPlease enter a second number: ";
            cin >> no2;
            
            switch (myoperator)
            {
                case '+': result = no1 + no2; break;
                case '-': result = no1 - no2; break;
                case '*': result = no1 * no2; break;
                case '/': result = no1 / no2; break;
            }
                
            cout << no1 << " " << myoperator << " " << no2
                 << " " << '=' << " " << result << "\n\n";
        }
        else
        {
            cout << "That operator has not been defined. Please try again.\n\n";
        }
    }

    return 0;
}

I should add that I agree with AceDawg45 regarding divide-by-zero errors. though I didn't include that in my code.

[TheToaster]

You should use stringstream defined in header <sstream>:

#include <sstream>
#include <iostream>

int main()
{
    double no1 = 0, no2 = 0, result = 0;
    char myoperator = 0;
    string BadInput //Protect from bad input
    
    

    cout << "This is a basic calculator. \n" << endl;

    start:
    cout << "Please enter a number: ";
    getline(cin, BadInput);
     
while (true) {
     stringstream ss(BadInput)
     if(ss>>no1) {
         break;
      }else{
     cerr << "ERROR: Bad input!
     goto start;
     } 

Also, you should limit your use of goto, using too many in your code makes it hard to change them and allows the creation of "spaghetti code".

[Chervil]

As for validating the input, this would probably be better as a separate function, but for now it can remain in the main code:

        cout << "Please enter a number: ";
        
        while ( ! (cin >> no1) )
        {
            cin.clear();            // reset the error flags
            cin.ignore(1000, '\n'); // empty input buffer
            
            cout << "Not numeric, Please enter a number: ";
        }

and to validate the operator, you might try this:

    const string valid_ops = "+-*/"; // list the valid characters in a string 

then check that the input is found in that string:

    cout << "Please choose an operator (+, -, *, /): ";
    cin >> myoperator;
    while (valid_ops.find(myoperator) == string::npos)
    {
        cout << "That operator has not been defined. Please try again. \n";
        cin >> myoperator;
    }

[Bogeyman]

Okay, this is what I have now;

#include "std_lib_facilities.h" // Header file made by Bjarne Stroustrup.

int main()
{
    double no1 = 0, no2 = 0, result = 0;
    char myoperator = ' ', yn = ' ';
    const string valid_ops = "+-*/"; // list the valid characters in a string

    cout << "This is a basic calculator. " << endl;
    start:
    cout << "\nPlease enter a number: ";
    cin >> no1;

    while ( ! (cin >> no1) )
    {
        cin.clear();            // reset the error flags
        cin.ignore(1000, '\n'); // empty input buffer

        cout << "Not numeric.\nPlease enter a number: ";
    }

    cout << "Please choose an operator (+, -, *, /): ";
    cin >> myoperator;

    while (valid_ops.find(myoperator) == string::npos)
    {
        cout << "Undefined operator.\nPlease choose an operator (+, -, *, /): ";
        cin >> myoperator;
    }

    if (myoperator == '+' || myoperator == '-' || myoperator == '*' || myoperator == '/')
    {
        cout << "Please enter a second number: ";
        cin >> no2;

        while ( ! (cin >> no2) )
        {
            cin.clear();            // reset the error flags
            cin.ignore(1000, '\n'); // empty input buffer

            cout << "Not numeric.\nPlease enter a number: ";
        }

        switch (myoperator)
        {
            case '+': result = no1 + no2; break;
            case '-': result = no1 - no2; break;
            case '*': result = no1 * no2; break;
            case '/': result = no1 / no2; break;
        }

        cout << no1 << " " << myoperator << " " << no2
        << " " << '=' << " " << result << "\n\n";
        cout << "Would you like to continue? Press 'y' to continue or 'n' to exit: ";
        ynstart:
        cin >> yn;
        if (yn == 'y')
        {
            goto start;
        }
        else if (yn == 'n')
        {
            goto progend;
        }
        else
        {
            cout << "Option not available. \n";
            goto ynstart;
        }

    }

    else
    {
        cout << "That operator has not been defined. Please try again.\n\n";
        goto start;
    }

    progend:

    return 0;
}

The problems I have now are that it requires two inputs for it to register a numeric value and I don't know how to correct a divide by zero error.

[Chervil]

The problems I have now are that it requires two inputs for it to register a numeric value

That's a misunderstanding. The only reason requires two inputs each time is because your code asks for two inputs.

In the above code, at both lines 12 and 14, you have cin >> no1. If you delete line 12 the problem is solved.

As for divide by zero, if the user enters myoperator == '/' you need to change the validation to check that the second number entered is not zero.

[Bogeyman]

Wow, what a silly mistake! Thanks Chervil, you've been a great help. The only thing that I need now is a divide-by-zero validation check. I have no idea how to do that.

[Chervil]

I have no idea how to do that.

Well, start by thinking of possible ways to approach the problem, then try to write some code to carry out your idea. If it isn't correct, then it may need some adjustment/modification, or maybe you need to think of another idea, and then try that.

[Bogeyman]

Alright, here is the final code for my basic calculator:

#include "std_lib_facilities.h" // Header file made by Bjarne Stroustrup.

int main()
{
    double no1 = 0, no2 = 0, result = 0;
    char myoperator = ' ', yn = ' ';
    const string valid_ops = "+-*/"; // list the valid characters in a string

    cout << "This is a basic calculator. " << endl;
    start:
    cout << "\nPlease enter a number: ";

    while ( ! (cin >> no1) )
    {
        cin.clear();            // reset the error flags
        cin.ignore(1000, '\n'); // empty input buffer

        cout << "Invalid input.\nPlease enter a number: ";
    }

    cout << "Please choose an operator (+, -, *, /): ";
    cin >> myoperator;

    while (valid_ops.find(myoperator) == string::npos)
    {
        cout << "Invalid input.\nPlease choose an operator (+, -, *, /): ";
        cin >> myoperator;
    }

    if (myoperator == '+' || myoperator == '-' || myoperator == '*' || myoperator == '/')
    {
        start2:
        cout << "Please enter a second number: ";

        while ( ! (cin >> no2) )
        {
            cin.clear();            // reset the error flags
            cin.ignore(1000, '\n'); // empty input buffer

            cout << "Invalid input.\nPlease enter a number: ";
        }

        switch (myoperator)
        {
            case '+': result = no1 + no2; break;
            case '-': result = no1 - no2; break;
            case '*': result = no1 * no2; break;
            case '/': result = no1 / no2; break;
        }

        if (myoperator == '/' && no2 == 0)
        {
            cout << "Cannot divide by 0.\n";
            goto start2;
        }
        else
        {
            cout << no1 << " " << myoperator << " " << no2
            << " " << '=' << " " << result << "\n\n";
        }

        cout << "Would you like to continue? Press 'y' to continue or 'n' to exit: ";
        ynstart:
        cin >> yn;
        if (yn == 'y')
        {
            goto start;
        }
        else if (yn == 'n')
        {
            goto progend;
        }
        else
        {
            cout << "Option unavailable. \n";
            goto ynstart;
        }

    }

    else
    {
        cout << "That operator has not been defined. Please try again.\n\n";
        goto start;
    }

    progend:

    return 0;
}

This has been a great exercise for me, and I have implemented everything I wanted, including user input validation checks. Thanks to everyone who helped, especially Chervil.

[Chervil]

You deserve some credit for getting this to work - though see comments below:

Your divide by zero check is done too late, after the divide has already been attempted. By then the program will already have crashed (depending on system and/or compiler).

Also, I see you're still using goto :( rather than a structured loop.
http://www.cplusplus.com/doc/tutorial/control/

If you've learned about functions, I'd suggest making the code which gets an integer into a separate function, then you could just do something like:

    no1 = getDouble();

or perhaps,

    no1 = getDouble("Enter first number");

it helps to keep the clutter out of the main program, as well as re-using code rather than typing the same code twice.
http://www.cplusplus.com/doc/tutorial/functions/

You might do the same with the code which gets an operator. Even though called only once, again it can reduce clutter. The aim is to have the main control flow readable in terms of showing clearly what is going on, without getting bogged down in the messy details.

An example of how the main() function might look with a lot of the detail handed over to other functions. This allows the entire thing to be viewed without scrolling, and also makes the looping structure clear. Notice how clear things can be without those goto statements which rather than making things simple, serve to obscure the structure.

int main()
{
    cout << "This is a basic calculator. \n\n";
            
    char yn = 'y';
    
    while (yn != 'n')
    {    
        double no1 = getDouble("Please enter first number: ");
    
        char myoperator = getOperator();

        cout << "The operator you have chosen is " << myoperator << endl;
             
        double no2 = getDouble("Please enter second number: ");
        
        while (myoperator == '/' && no2 == 0)
        {       
            cout << "Cannot divide by Zero. ";
            no2 = getDouble("Please enter second number: ");            
        }
        
        double result = calculate(no1, no2, myoperator);
        
        cout << no1 << ' ' << myoperator << ' ' << no2 << " = " << result << "\n\n";
                  
        cout << "Would you like to continue?\n";        
        do {
            cout << "Press 'y' to continue or 'n' to exit: ";
            yn = tolower(cin.get());
        } while (yn != 'y' && yn != 'n');  
    }
}

[Bogeyman]

Unfortunately at the moment I have not learned about functions, though I do see how correct implementation can serve to streamline and increase legibility of code. I have only been coding for a while now, so not everything you have shown me makes perfect sense. However, I appreciate the effort you have put into helping me. I still have much to learn!

[Chervil]

No problem, you're welcome. I do understand that you won't be able to take in everything all in one go, it takes time. Following things at the pace of your book is a good plan.