C++ program stuck in an infinite loop

Question

Please note that I am a complete beginner at C++. I'm trying to write a simple program for an ATM and I have to account for all errors. User may use only integers for input so I need to check if input value is indeed an integer, and my program (this one is shortened) works for the most part.

The problem arises when I try to input a string value instead of an integer while choosing an operation. It works with invalid value integers, but with strings it creates an infinite loop until it eventually stops (unless I add system("cls"), then it doesn't even stop), when it should output the same result as it does for invalid integers:

Invalid choice of operation.
Please select an operation:

 1 - Balance inquiry
 7 - Return card

Enter your choice and press return:

Here is my code:

#include <iostream>
#include <string>
using namespace std;


bool isNumber(string s) //function to determine if input value is int
{
    for (int i = 0; i < s.length(); i++)
        if (isdigit(s[i]) == false)
            return false;
    return true;
}


int ReturnCard() //function to determine whether to continue running or end program
{
    string rtrn;

    cout << "\nDo you wish to continue? \n1 - Yes \n2 - No, return card" << endl;
    cin >> rtrn;
    if (rtrn == "1" and isNumber(rtrn)) { return false; }
    else if (rtrn == "2" and isNumber(rtrn)) { return true; }
    else {cout << "Invalid choice." << endl; ReturnCard(); };
    return 0;
}


int menu() //function for operation choice and execution
{
    int choice;
    do
    {
        cout << "\nPlease select an operation:\n" << endl
            << " 1 - Balance inquiry\n"
            << " 7 - Return card\n"
            << "\nEnter your choice and press return: ";
        int balance = 512;
        cin >> choice;
        if (choice == 1 and isNumber(to_string(choice))) { cout << "Your balance is $" << balance; "\n\n"; }
        else if (choice == 7 and isNumber(to_string(choice))) { cout << "Please wait...\nHave a good day." << endl; return 0; }
        else { cout << "Invalid choice of operation.";  menu(); }
    } while (ReturnCard()==false);
    cout << "Please wait...\nHave a good day." << endl;
    return 0;
}

int main()
{
    string choice;
    cout << "Insert debit card to get started." << endl;
    menu();
    return 0;
}

I've tried every possible solution I know, but nothing seems to work.

***There is a different bug, which is that when I get to the "Do you wish to continue?" part and input any invalid value and follow it up with 2 (which is supposed to end the program) after it asks again, it outputs the result for 1 (continue running - menu etc.). I have already emailed my teacher about this and this is not my main question, but I would appreciate any help.

Thank you!

Answer 1

It is always good to save console input in a string variable instead of another type, e.g. int or double. This avoids trouble with input errors, e.g. if characters instead of numbers are given by the program user. Afterwards the string variable could by analyzed for further actions.
Therefore I changed the type of choice from int to string and adopted the downstream code to it.

Please try the following program and consider my adaptations which are written as comments starting with tag //CKE:. Thanks.

#include <iostream>
#include <string>
using namespace std;


bool isNumber(const string& s) //function to determine if input value is int
{
    for (size_t i = 0; i < s.length(); i++) //CKE: keep same variable type, e.g. unsigned
        if (isdigit(s[i]) == false)
            return false;
    return true;
}


bool ReturnCard() //function to determine whether to continue running or end program
{
    string rtrn;

    cout << "\nDo you wish to continue? \n1 - Yes \n2 - No, return card" << endl;
    cin >> rtrn;
    if (rtrn == "1" and isNumber(rtrn)) { return false; }
    if (rtrn == "2" and isNumber(rtrn)) { return true; } //CKE: remove redundant else
    cout << "Invalid choice." << endl; ReturnCard(); //CKE: remove redundant else + semicolon
    return false;
}


int menu() //function for operation choice and execution
{
    string choice; //CKE: change variable type here from int to string
    do
    {
        cout << "\nPlease select an operation:\n" << endl
            << " 1 - Balance inquiry\n"
            << " 7 - Return card\n"
            << "\nEnter your choice and press return: ";
        int balance = 512;
        cin >> choice;
        if (choice == "1" and isNumber(choice)) { cout << "Your balance is $" << balance << "\n\n"; } //CKE: semicolon replaced by output stream operator
        else if (choice == "7" and isNumber(choice)) { cout << "Please wait...\nHave a good day." << endl; return 0; }
        else { cout << "Invalid choice of operation."; } //CKE: remove recursion here as it isn't required
    } while (!ReturnCard()); //CKE: negate result of ReturnCard function
    cout << "Please wait...\nHave a good day." << endl;
    return 0;
}

int main()
{
    string choice;
    cout << "Insert debit card to get started." << endl;
    menu();
    return 0;
}

Answer 2

There are a few things mixed up in your code. Always try to compile your code with maximum warnings turned on, e.g., for GCC add at least the -Wall flag. Then your compiler would warn you of some of the mistakes you made.

First, it seems like you are confusing string choice and int choice. Two different variables in different scopes. The string one is unused and completely redundant. You can delete it and nothing will change.

In menu, you say cin >> choice;, where choice is of type int. The stream operator >> works like this: It will try to read as many characters as it can, such that the characters match the requested type. So this will only read ints.

Then you convert your valid int into a string and call isNumber() - which will alway return true.

So if you wish to read any line of text and handle it, you can use getline():

string inp;
std::getline(std::cin, inp);
if (!isNumber(inp)) {
    std::cout << "ERROR\n";
    return 1;
}

int choice = std::stoi(inp); // May throw an exception if invalid range

See stoi

Your isNumber() implementation could look like this:

#include <algorithm>

bool is_number(const string &inp) {
  return std::all_of(inp.cbegin(), inp.cend(), 
    [](unsigned char c){ return std::isdigit(c); });
}

If you are into that functional style, like I am ;)

EDIT:

Btw., another bug which the compiler warns about: cout << "Your balance is $" << balance; "\n\n"; - the newlines are separated by ;, so it's a new statement and this does nothing. You probably wanted the << operator instead.

Recursive call bug:

In { cout << "Invalid choice of operation."; menu(); } and same for ReturnCard(), the function calls itself (recursion). This is not at all what you want! This will start the function over, but once that call has ended, you continue where that call happened. What you want in menu() is to start the loop over. You can do that with the continue keyword. You want the same for ReturnCard(). But you need a loop there. And now, that I read that code, you don't even need to convert the input to an integer. All you do is compare it. So you can simply do:

string inp;
std::getline(std::cin, inp);
if (inp == "1" || inp == "2") {
  // good
} else {
  // Invalid
}

Unless that is part of your task.