HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

Checking if each char in a string is a decimal digit

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
eachcharcheckingdecimaldigitstring

Problem

I had to write a program that would verify that a string contains any digits beside numbers 0-9 and the first '-' sign to indicate if pos/neg. The algorithm works like I want it to, but is there a better way of writing it, e.g. streamlining it, making it shorter?

bool isNum(string number) {
    // Checks if first char in buffer contains invalid digits 
    // Needed because any negative sign after the one used to indicate negative sign.
    if (!((number[0] >= '0' && number[0]  '9')
            return false;
    }
    // If neither statement returns false, returns true by default
    return true;
}

Solution

-
In the cctype header, you have the std::isdigit(int) function. You can use this instead of your conditions that check if the character is between '0' and '9'.

-
You index into a std::string with an int inside the for loop. Use std::string::size_type as that is the proper type that can index into a std::string (int might be too small).

-
Mark the function as noexcept.

-
Consider passing the std::string parameter by reference if you're planning on using large number strings (~20 character numbers). This prevents unnecessary copying from dynamically allocated strings.

-
Check vs. the size instead of indexing into the string and checking if the current character is '\0'.

-
Qualify standard types with std::. Don't use using namespace std;.

-
There is no need to negate that first condition.

-
Bug: isNum("-") and isNum("+") return true. An additional check is required if the first character is '+' or '-'.

-
Naming is important. Your function is called isNum(), but I believe that isInt() better describes it; the function does not detect floating point numbers. Please also consider this comment by Toby Speight in regards to naming.

After changes are applied:

#include 
#include 

bool isinteger(std::string const& n) noexcept
{
    if (std::isdigit(n[0]) || (n.size() > 1 && (n[0] == '-' || n[0] == '+')))
    {
        for (std::string::size_type i{ 1 }; i < n.size(); ++i)
            if (!std::isdigit(n[i]))
                return false;

        return true;
    }
    return false;
}


Note 1: I've renamed the function to isinteger() to keep consistency with other similar functions such as isdigit(). I've also reordered the initial condition to take advantage of boolean short circuiting.

Note 2: As Rakete1111 points out, if your compiler hasn't caught up to C++11 yet, accessing the first character of an empty string is undefined behaviour.

The condition should be modified like so:

// check that the string is not empty, only if we're working with  1 && (n[0] == '-' || n[0] == '+')))
{
    // ...
}


However, if your compiler is C++11 (or later) compliant, you can safely leave it; accessing the first character of empty strings returns '\0' in that case.

See here for more information: http://en.cppreference.com/w/cpp/string/basic_string/operator_at

Code Snippets

#include <string>
#include <cctype>

bool isinteger(std::string const& n) noexcept
{
    if (std::isdigit(n[0]) || (n.size() > 1 && (n[0] == '-' || n[0] == '+')))
    {
        for (std::string::size_type i{ 1 }; i < n.size(); ++i)
            if (!std::isdigit(n[i]))
                return false;

        return true;
    }
    return false;
}
// check that the string is not empty, only if we're working with < C++11
if ((!n.empty() && std::isdigit(n[0])) || (n.size() > 1 && (n[0] == '-' || n[0] == '+')))
{
    // ...
}

Context

StackExchange Code Review Q#162569, answer score: 19

Revisions (0)

No revisions yet.