patterncppMinor
Determining if an std::string contains a numerical type
Viewed 0 times
stddeterminingnumericaltypecontainsstring
Problem
I recently came across this SO question asking for methods to determine if a
Here's an example/expected behaviour:
```
#include
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
s
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
#include
#include
#include
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
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
s
Solution
Functional issues
Your code doesn't consider negative numbers to be integral. You're also missing support for things like
and similar for floating point.
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:
There's actually no reason to bring this in, since you could already make an unqualified call to
Too much action
This is really busy:
And it's confusing since you increment
Since there's no modification going on here, each of the 3 parts are easier to reason about.
Just
A logical extension would be to change your functions to take iterator pairs, or
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 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_digitThere'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
bool is_integral(It first, It last);
// ... and a helper to forward
bool is_integral(const std::string& str);Code Snippets
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;
}
}const auto is_digit = [] (const char c) { return std::isdigit(c); };
bool is_digit(char c) { return std::isdigit(c); }return it != cend(str) && *it++ == '.' && std::all_of(it, cend(str), detail::is_digit);return it != cend(str) &&
*it == '.' &&
std::all_of(it + 1, cend(str), detail::is_digit);// 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);Context
StackExchange Code Review Q#115117, answer score: 5
Revisions (0)
No revisions yet.