1

I'm looking for a short (and fast) code to check whether a string contains only digits, specifically looking for a one liner. This is my temporary code:

bool IsNumber(const std::string& str)
{
    int i = 0;
    for( ; i<str.size() && isdigit(str[i]); ++i);

    return ( i == str.size() );
}
W2a
  • 736
  • 2
  • 9
  • 23

1 Answers1

6

Use std::all_of along with isdigit:

#include <algorithm>
#include <cctype>
//..
bool allDigits = (!str.empty() && std::all_of(str.begin(), str.end(), ::isdigit));

Edit: Added check for an empty string.

PaulMcKenzie
  • 34,698
  • 4
  • 24
  • 45
  • 1
    What if the string is empty? – rustyx Feb 12 '17 at 19:29
  • Very nice; I suggest the use of `cbegin()` and `cend()` instead of `begin()`/`end()` (but not sure it's usefull) – max66 Feb 12 '17 at 19:30
  • 2
    @ChristianHackl - `std::all_of` returns `true` for an empty range. – Pete Becker Feb 12 '17 at 19:33
  • @RustyX: I could have sworn it returned `false`, because that would make more sense to me, but to my astonishment it returns `true` in that case. – Christian Hackl Feb 12 '17 at 19:35
  • Just add && !str.empty() to the line – Alex Zywicki Feb 12 '17 at 19:37
  • 1
    @AlexZywicki -- Better to check for the empty string first. This way the `&&` becomes short-circuited if the string is empty, instead of going into the `std::all_of`. – PaulMcKenzie Feb 12 '17 at 19:39
  • @PaulMcKenzie good catch – Alex Zywicki Feb 12 '17 at 19:41
  • 1
    Unfortunately, in order to be portable and avoid undefined behavior, you need to cast the argument of `isdigit` to `unsigned char`, which would require adding a lambda and making this code much uglier. – interjay Feb 12 '17 at 19:42
  • The reason `all_of` returns true for an empty range is that it would be an extra test to return false. That would reduce performance in situations where it doesn't matter. – rustyx Feb 12 '17 at 19:43
  • 1
    @RustyX: I would argue that it's not much a matter of performance, but logic: the sum of an empty range is 0 (the neutral value for sum) and the product of an empty range is 1 (neutral value of product); `all_of` is repeated `and`, and the neutral element of `and` is `true`. The fact that it saves a branch in the implementation is just a consequence of the "obviousness" of this choice. – Matteo Italia Feb 12 '17 at 19:48
  • 2
    @RustyX It isn't due to performance. In logic, for an empty set it is always true that a predicate holds for all of its elements: see https://en.wikipedia.org/wiki/Vacuous_truth. – interjay Feb 12 '17 at 19:48
  • @interjay: OTOH, if you use a lambda anyway, then you can easily turn this into a `std::any_of` call with a negated predicate (`!isdigit`) and don't need the `empty()` check anymore. – Christian Hackl Feb 13 '17 at 08:47
  • i.e. `!std::any_of(begin(s), end(s), [](char c) { return !::isdigit(static_cast(c)); });` - whether that's better than before or not, I don't know. I guess I'd wrap it in a `bool all_digits(std::string const& s)` function anyway :) – Christian Hackl Feb 13 '17 at 09:08
  • @ChristianHackl That would return true for an empty string, just like omitting the `empty` check in the `all_of` version. – interjay Feb 13 '17 at 14:09