Checking if command line input is an integer

Question

The code below accepts 1 command line argument from the user. The expected input is an integer. Hence, the code checks if the input is purely an integer. Any feedback is appreciated.

atoi() does not provide error-checking, thus the idea is to verify the input before passing the input to atoi().

I considered using strtol(), but it looks way too complex for a simple task.

stoi() considers 1a as valid, which is not valid in my use case.

//abc   ->  Invalid
//1a    ->  Invalid
//a1    ->  Invalid
//1     ->  Valid 

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

using namespace std;

int correctInput;
string userInputStr;

int main(int argc, char* argv[])
{
    userInputStr = argv[1];
    for ( int x = 0; x < userInputStr.length(); x++)
    {
        if ( isalpha(userInputStr[x]) )
        {
            cout << "invalid input." << endl;
            return 0;
        }

    }

    correctInput = atoi(userInputStr.c_str());

    cout << correctInput << endl;

}

Answer 1

strtol will work just fine for this job:

char *end;
int correctInput = strtol(argv[1], &end, 10);

if (*end != '\0') {
    std::cout << "invalid input.\n";
    return 1;
}
std::cout << correctInput << "\n";

That said, let's do a quick review of your existing code.

Avoid using namespace std;

This is common in quite a few tutorials and such, largely of the sake of brevity. In any case, it's generally best avoided in real code.

Avoid global variables

Right now, you've defined correctInput and userInputString as global variables, even though they're only used inside of main. As a general rule of thumb, you want to restrict variables to the smallest scope necessary to do their job.

Prefer initialization to assignment

In this case, moving userInputString inside of main would have one more beneficial effect: it's better to initialize than to create an uninitialized variable (a string, in this case) and assigning a value to it later. I'd prefer to use:

std::string userInputString(argv[1]);

...or:

std::string userInputString{argv[1]};

This means that userInputString contains the correct value as soon as it exists.

Check argc before using arguments

You always want to check that argv[1] actually exists before trying to use it (and, by strong preference, print out a meaningful error message if it doesn't):

if (argc < 2) {
    std::cerr << "Usage: check <int>\n';
    return 1;
}

This tends to be more helpful than the default error message (e.g., Segmentation fault (core dumped)).

Use meaningful return values

You return 0 from main (indicating success) regardless of the input. You't typically want to indicate failure when the input was bad:

        cout << "invalid input." << endl;
        return EXIT_FAILURE;

Avoid std::endl

std::endl flushes the output stream, which is rarely needed, and tends to be undesirable. In this particular case it doesn't make much difference, but I'd get in the habit of using '\n' when you want to print a new-line, so you won't accidentally use std::endl when you're producing enough output for it to make a real difference.

Use standard algorithms where applicable

If you do decide to continue checking that the input is all digits, it's probably better to use std::all_of (or perhaps, std::any_of) instead of writing a loop to do the job:

std::string input(argv[1]);

if (std::any_of(input.begin(), input.end(), [](char ch) { return !isdigit(ch); }) {
    cout << "invalid input." << endl;
    return EXIT_FAILURE; 
}