3
\$\begingroup\$

I recently came across this SO question asking for methods to determine if a std::string represents an integer. There is also this SO question for floats. I decided to code up a few methods consolidating these questions.

#include <string>
#include <algorithm>
#include <iterator>
#include <cctype>

namespace detail
{
    const auto is_digit = [] (const char c) { return std::isdigit(c); };
}

bool is_integral(const std::string& str)
{
    return !str.empty() && std::all_of(std::cbegin(str), std::cend(str), detail::is_digit);
}

bool is_floating_point(const std::string& str)
{
    using std::cbegin; using std::cend;

    auto it = std::find_if_not(cbegin(str), cend(str), detail::is_digit);

    return it != cend(str) && *it++ == '.' && std::all_of(it, cend(str), detail::is_digit);
}

bool is_arithmetic(const std::string& str)
{
    using std::cbegin; using std::cend;

    if (str.empty()) return false;

    auto it = std::find_if_not(cbegin(str), cend(str), detail::is_digit);

    return it == cend(str) || (*it++ == '.' && std::all_of(it, cend(str), detail::is_digit));
}

Here's an example/expected behaviour:

#include <iostream>

int main()
{
    std::string empty {};
    std::string integer {"123"};
    std::string floating1 {"123."};
    std::string floating2 {"123.4"};
    std::string neither1 {"123..4"};
    std::string neither2 {"1r23"};

    std::cout << std::boolalpha;

    std::cout << is_integral(empty) << std::endl; // false
    std::cout << is_floating_point(empty) << std::endl; // false
    std::cout << is_arithmetic(empty) << std::endl; // false
    std::cout << std::endl;

    std::cout << is_integral(integer) << std::endl; // true
    std::cout << is_floating_point(integer) << std::endl; // false
    std::cout << is_arithmetic(integer) << std::endl; // true
    std::cout << std::endl;

    std::cout << is_integral(floating1) << std::endl; // false
    std::cout << is_floating_point(floating1) << std::endl; // true
    std::cout << is_arithmetic(floating1) << std::endl; // true
    std::cout << std::endl;

    std::cout << is_integral(floating2) << std::endl; // false
    std::cout << is_floating_point(floating2) << std::endl; // true
    std::cout << is_arithmetic(floating2) << std::endl; // true
    std::cout << std::endl;

    std::cout << is_integral(neither1) << std::endl; // false
    std::cout << is_floating_point(neither1) << std::endl; // false
    std::cout << is_arithmetic(neither1) << std::endl; // false
    std::cout << std::endl;

    std::cout << is_integral(neither2) << std::endl; // false
    std::cout << is_floating_point(neither2) << std::endl; // false
    std::cout << is_arithmetic(neither2) << std::endl; // false
    std::cout << std::endl;

    std::cout << is_integral(neither2) << std::endl; // false
    std::cout << is_floating_point(neither2) << std::endl; // false
    std::cout << is_arithmetic(neither2) << std::endl; // false
    std::cout << std::endl;

    return 0;
}

I'm looking for any improvements.

\$\endgroup\$
7
  • \$\begingroup\$ What about negative numbers? \$\endgroup\$ Commented Dec 26, 2015 at 15:17
  • \$\begingroup\$ What about hexadecimal integers ? \$\endgroup\$
    – user24597
    Commented Dec 26, 2015 at 15:20
  • \$\begingroup\$ oh dear, forgot about negative numbers! I was thinking about hexadecimal, but decided not to in the interest of performance. \$\endgroup\$
    – Daniel
    Commented Dec 26, 2015 at 15:25
  • \$\begingroup\$ @Daniel Also std::stoi(), std::stod(), etc. already are available for this functionality. \$\endgroup\$ Commented Dec 26, 2015 at 15:27
  • 2
    \$\begingroup\$ @Daniel The pos parameter can be checked how many characters were actually processed. \$\endgroup\$ Commented Dec 26, 2015 at 15:33

2 Answers 2

5
\$\begingroup\$

Functional issues

Your code doesn't consider negative numbers to be integral. You're also missing support for things like 1e4 for floating point. Ultimately, std::stoi and std::stof might be better for your needs:

boost::optional<int> convert_integer(std::string const& str) {
    try {
        size_t pos;
        int result = std::stoi(str, &pos);
        if (pos == str.size()) {
            return result;
        }
        else {
            return boost::none;
        }
    }
    catch (std::invalid_argument& ) {
        return boost::none;
    }
    catch (std::out_of_range& ) {
        return boost::none;
    }
} 

and similar for floating point.

is_digit

There's no reason for this to be a lambda. It's actually quite a bit shorter if you just write it as a function:

const auto is_digit = [] (const char c) { return std::isdigit(c); };
bool is_digit(char c) { return std::isdigit(c); }

using std::cbegin;

There's actually no reason to bring this in, since you could already make an unqualified call to cbegin() thanks to ADL. Also, you don't even need cbegin() since begin() would have done the same thing (str is a reference to const in all of your functions).

Too much action

This is really busy:

return it != cend(str) && *it++ == '.' && std::all_of(it, cend(str), detail::is_digit);

And it's confusing since you increment it in the middle. Which happens to work, but I had to stop and really think about it to convince myself that it does. You could simply shift the ++ into the all_of:

return it != cend(str) && 
    *it == '.' && 
    std::all_of(it + 1, cend(str), detail::is_digit);

Since there's no modification going on here, each of the 3 parts are easier to reason about.

Just std::string?

A logical extension would be to change your functions to take iterator pairs, or const char* pairs. You could provide convenience string methods to forward to the helpers, but it'd be potentially more useful to have

// one of...
bool is_integral(const char* first, const char* last);
template <class It>
bool is_integral(It first, It last);

// ... and a helper to forward
bool is_integral(const std::string& str);
\$\endgroup\$
2
  • \$\begingroup\$ You cheated a bit with the 2 const in the lambda vs function length comparison. \$\endgroup\$
    – nwp
    Commented Dec 26, 2015 at 18:20
  • \$\begingroup\$ @nwp With the best intentions though! \$\endgroup\$
    – Barry
    Commented Dec 26, 2015 at 20:05
1
\$\begingroup\$

An alternative approach for strings that may be convertible to long would be this:

bool is_integral(const std::string& str) {
    bool islong = true;
    try { 
        std::stol(str);
    } catch(...) {
        islong = false;
    }
    return islong;
}

For floating point values, there is a corresponding stod that will throw if the passed string is not convertible.

\$\endgroup\$
2
  • \$\begingroup\$ This is insufficient and will return true for something like "1s" \$\endgroup\$
    – Barry
    Commented Dec 26, 2015 at 20:04
  • \$\begingroup\$ @Barry: Insufficient by what definition? That is the behavior I would want and expect. If it could be interpreted as a number, return true. \$\endgroup\$
    – Edward
    Commented Dec 26, 2015 at 20:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.