Integer validation in C++

Question

I am studying C++ and after I learned about some functions of the library cctype like isdigit I decided to make a program that validates user input only to be an integer number. So input like 123as, :$ or +.234 are invalid, however I decided to keep the plus or minus sign, i.e., input like -24 or +142 are valid. Below is my code:

#include <iostream> 
#include <string>
#include <cctype>

using namespace std;

bool integer_valid(string s);
int get_integer();

int main()
{
    int number;

    number = get_integer();
    cout << "The integer number is:\t" << number << "\n" << endl;

    system("pause");
    return 0;
}

bool integer_valid(string s)
{
    int i, length_s;
    bool is_valid = true;

    length_s = s.length();

    if (length_s == 0)
        is_valid = false;
    else
    {
        if (length_s == 1)
        {
            if (!isdigit(s[0]))
                is_valid = false;           
        }
        else
        {
            if (s[0] == '+' || s[0] == '-' || isdigit(s[0]) != 0)
            {
                for (i = 1; i < length_s; i++)
                {
                    if (!isdigit(s[i]))
                        is_valid = false;
                }                               
            }
            else
                is_valid = false;
        }           
    }
    return is_valid;
}

int get_integer()
{
    int new_number;
    string number;
    bool is_valid;

    do {
        cout << "Enter an integer number:\t";
        getline(cin, number);
        cout << "\n";

        is_valid = integer_valid(number);

        if (is_valid == false)
            cout << "ERROR. The entered number must be an integer.\n" << endl;
    } while (is_valid == false);

    new_number = stoi(number);

    return new_number;
}

I'm aware that in general is a bad practice to use using namespace std, but since my program has less tan 100 lines of code I think it's really not a big issue here. Moreover, I wonder if there is another (simpler/faster) way to validate integer numbers?

EDIT: Answering to the comment made by Roland Illig, I would say yes. More exactly, any kind of integer should be valid, but as far as I know, int has a defined size, so if I want to valid long integers I should use the type long long int and also the function stoll, right?

Any feedback would be greatly appreciated.

Answer 1

• Using a bool is_valid results in a deeply nested code. You have 5 levels of nesting, with the core of algorithm being hidden at the deepest one. I recommend an early return: as soon as you determine that the string doesn't pass a criteria, return false; immediately.

• Testing for length_s == 0 is redundant. In this case s[0] is '\0', which is neither a digit nor a sign.

• Passing std::string as an argument makes a copy. Prefer passing a (constant) reference.

• Leaving the point of the exercise aside, the best way to validate a number is to just go ahead and directly call std:stoi with the second parameter, e.g.:

      size_t end;
      int number = std::stoi(s, &end);
      if (s[end] != '\0') {
          // The string was not parsed completely. It is not a number.
      }
  

Answer 2

Avoid system("pause")

It's a security risk, not portable, and introduces unnecessary overhead.

Variables should use the proper types

The return type for std::string::length() is std::string::size_type (which is usually an unsigned type), so that should be the type for length_s, not int.

Similarly, i should be of type std::string::size_type because:

1. you are comparing it to length_s on the line for (i = 1; i < length_s; i++). The two variables should both be unsigned for proper comparison, and preferably the exact same type.
2. you are using it as the argument type for std::string::operator[], which uses std::string::size_type as the argument type.

I would still avoid using namespace std

On Stack Overflow, I once saw an even shorter program which failed to compile due to using namespace std -- the programmer attempted to define a function int plus(int, int) which conflicted with std::plus. Your program may have less than 100 lines of code, but that just means there aren't too many instances in which you have to qualify a name with std::. Plus, it's best to get into the habit of not using using namespace std for when you write longer programs.

Answer 3

Solution one:

You're already using std::stoi, did you know that it throws an exception if it can't convert to an int? It already does the validation for you, use it like:

do {
    ... read input ...

    try {
        number = std::stoi(s);
    } catch (const std::exception& e) {
        std::cout << "ERROR. The entered number must be an integer.\n";
        valid = false;
    }
} while (!valid);

Solution two:

If you just check if (std::cin >> number) (or in the condition of a loop) it'll fail if it couldn't read an int. No extra validation needed.

Other points:

• Use \n instead of std::endl unless you explicitly need to flush the stream (you don't)
• Never use using namespace std, even in short programs
• Check a bool using myBool == false is strange to read, prefer !myBool