8
\$\begingroup\$

This is a follow-up question for is_number Function Implementation in C++. Considering G. Sliepen's answer and Harith's answer, I want to focus on decimal number here, therefore, is_decimal function implementation is presented in this post.

The experimental implementation

  • is_decimal function implementation

    constexpr bool is_decimal(std::string_view s)
    {
        if(s.empty())
        {
            return false;
        }
        bool is_negative_number = s.starts_with("-");
        bool is_positive_number = s.starts_with("+");
        return ((is_decimal_helper(s.substr(is_negative_number, s.size())) ||
                is_decimal_helper(s.substr(is_positive_number, s.size()))) &&
                std::ranges::count(s, '.') <= 1) ||
               (is_scientific_notation(s));
    }
    
  • is_decimal_helper function implementation

    //  Modified from https://stackoverflow.com/a/4654718/6667035
    constexpr bool is_decimal_helper(std::string_view s)
    {
        auto it = std::ranges::begin(s);
        while (it != std::ranges::end(s) && (std::isdigit(static_cast<unsigned char>(*it)) || *it == '.')) ++it;
        return !s.empty() && it == std::ranges::end(s);
    }
    
  • is_scientific_notation function implementation

    //  From: https://stackoverflow.com/a/37198619/6667035
    constexpr bool is_scientific_notation(std::string_view input)
    {
        int state = 0;
        for (std::string::size_type i = 0; i < input.size(); ++i) {
            char character = input.at(i);
            switch(state) {
                case 0: {
                    // state 0: accept on '. or' '-' or digit
                    if (character == '.') {
                        state = 3;
                    } else if (character == '-') {
                        state = 1;
                    } else if (std::isdigit(character)) {
                        state = 2;
                    } else {
                        return false;
                    }
                    break;
                }
                case 1: {
                    // state 1: accept on '. or digit
                    if (character == '.') {
                        state = 3;
                    } else if (std::isdigit(character)) {
                        state = 2;
                    } else {
                        return false;
                    }
                    break;
                }
                case 2: {
                    // state 2: accept on '.' or 'e' or 'E' digit
                    if (character == '.') {
                        state = 4;
                    } else if ((character == 'e') || (character == 'E')) {
                        state = 5;
                    } else if (std::isdigit(character)) {
                        state = 2;
                    } else {
                        return false;
                    }
                    break;
                }
                case 3: {
                    // state 3: accept on digit
                    if (std::isdigit(character)) {
                        state = 4;  
                    } else {
                        return false;
                    }
                    break;
                }
                case 4: {
                    // state 4: accept on 'e' or 'E' or digit
                    if ((character == 'e') || (character == 'E')) {
                        state = 5;
                    } else if (std::isdigit(character)) {
                        state = 4;  
                    } else {
                        return false;
                    }
                    break;
                }
                case 5: {
                    // state 5: accept on '+' or '-' or digit
                    if ((character == '+') || (character == '-')) {
                        state = 6;
                    } else if (std::isdigit(character)) {
                        state = 6;  
                    } else {
                        return false;
                    }
                    break;
                }
                case 6: {
                    // state 6: accept on digit
                    if (std::isdigit(character)) {
                        state = 6;  
                    } else {
                        return false;
                    }
                    break;
                }
            }
        }
        if (state == 6) {
            return true;
        } else {
            return false;
        }
    }
    

Full Testing Code

The full testing code:

//  is_decimal Function Implementation in C++

#include <algorithm>
#include <cassert>
#include <cctype>
#include <chrono>
#include <concepts>
#include <iostream>
#include <numeric>
#include <ranges>
#include <string>

//  Copy from https://stackoverflow.com/a/37264642/6667035
#ifndef NDEBUG
#   define M_Assert(Expr, Msg) \
    M_Assert_Helper(#Expr, Expr, __FILE__, __LINE__, Msg)
#else
#   define M_Assert(Expr, Msg) ;
#endif

void M_Assert_Helper(const char* expr_str, bool expr, const char* file, int line, const char* msg)
{
    if (!expr)
    {
        std::cerr << "Assert failed:\t" << msg << "\n"
            << "Expected:\t" << expr_str << "\n"
            << "Source:\t\t" << file << ", line " << line << "\n";
        abort();
    }
}

//  From: https://stackoverflow.com/a/37198619/6667035
constexpr bool is_scientific_notation(std::string_view input)
{
    int state = 0;
    for (std::string::size_type i = 0; i < input.size(); ++i) {
        char character = input.at(i);
        switch(state) {
            case 0: {
                // state 0: accept on '. or' '-' or digit
                if (character == '.') {
                    state = 3;
                } else if (character == '-') {
                    state = 1;
                } else if (std::isdigit(character)) {
                    state = 2;
                } else {
                    return false;
                }
                break;
            }
            case 1: {
                // state 1: accept on '. or digit
                if (character == '.') {
                    state = 3;
                } else if (std::isdigit(character)) {
                    state = 2;
                } else {
                    return false;
                }
                break;
            }
            case 2: {
                // state 2: accept on '.' or 'e' or 'E' digit
                if (character == '.') {
                    state = 4;
                } else if ((character == 'e') || (character == 'E')) {
                    state = 5;
                } else if (std::isdigit(character)) {
                    state = 2;
                } else {
                    return false;
                }
                break;
            }
            case 3: {
                // state 3: accept on digit
                if (std::isdigit(character)) {
                    state = 4;  
                } else {
                    return false;
                }
                break;
            }
            case 4: {
                // state 4: accept on 'e' or 'E' or digit
                if ((character == 'e') || (character == 'E')) {
                    state = 5;
                } else if (std::isdigit(character)) {
                    state = 4;  
                } else {
                    return false;
                }
                break;
            }
            case 5: {
                // state 5: accept on '+' or '-' or digit
                if ((character == '+') || (character == '-')) {
                    state = 6;
                } else if (std::isdigit(character)) {
                    state = 6;  
                } else {
                    return false;
                }
                break;
            }
            case 6: {
                // state 6: accept on digit
                if (std::isdigit(character)) {
                    state = 6;  
                } else {
                    return false;
                }
                break;
            }
        }
    }
    if (state == 6) {
        return true;
    } else {
        return false;
    }
}

//  Modified from https://stackoverflow.com/a/4654718/6667035
constexpr bool is_decimal_helper(std::string_view s)
{
    auto it = std::ranges::begin(s);
    while (it != std::ranges::end(s) && (std::isdigit(static_cast<unsigned char>(*it)) || *it == '.')) ++it;
    return !s.empty() && it == std::ranges::end(s);
}

constexpr bool is_decimal(std::string_view s)
{
    if(s.empty())
    {
        return false;
    }
    bool is_negative_number = s.starts_with("-");
    bool is_positive_number = s.starts_with("+");
    return ((is_decimal_helper(s.substr(is_negative_number, s.size())) ||
            is_decimal_helper(s.substr(is_positive_number, s.size()))) &&
            std::ranges::count(s, '.') <= 1) ||
           (is_scientific_notation(s));
}

void is_decimal_test()
{
    M_Assert(
        is_decimal("") == false,
        "Empty string test case failed");

    M_Assert(
        is_decimal("123") == true,
        "123 test case failed");

    M_Assert(
        is_decimal("123.123") == true,
        "123.123 test case failed");

    M_Assert(
        is_decimal("-123") == true,
        "-123 test case failed");

    M_Assert(
        is_decimal("-123.123") == true,
        "-123.123 test case failed");

    M_Assert(
        is_decimal("0") == true,
        "0 test case failed");

    M_Assert(
        is_decimal("-0") == true,
        "-0 test case failed");

    M_Assert(
        is_decimal("2147483647") == true,
        "2147483647 test case failed");

    M_Assert(
        is_decimal("-2147483648") == true,
        "-2147483648 test case failed");

    //  values from https://stackoverflow.com/a/48650578/6667035
    M_Assert(
        is_decimal("-340282346638528859811704183484516925440.0000000000000000") == true,
        "-340282346638528859811704183484516925440.0000000000000000 test case failed");

    M_Assert(
        is_decimal("340282346638528859811704183484516925440.0000000000000000") == true,
        "340282346638528859811704183484516925440.0000000000000000 test case failed");

    M_Assert(
        is_decimal("-179769313486231570814527423731704356798070567525844996598917476803157260780028538760589558632766878171540458953514382464234321326889464182768467546703537516986049910576551282076245490090389328944075868508455133942304583236903222948165808559332123348274797826204144723168738177180919299881250404026184124858368.0000000000000000") == true,
        "-179769313486231570814527423731704356798070567525844996598917476803157260780028538760589558632766878171540458953514382464234321326889464182768467546703537516986049910576551282076245490090389328944075868508455133942304583236903222948165808559332123348274797826204144723168738177180919299881250404026184124858368.0000000000000000 test case failed");

    M_Assert(
        is_decimal("179769313486231570814527423731704356798070567525844996598917476803157260780028538760589558632766878171540458953514382464234321326889464182768467546703537516986049910576551282076245490090389328944075868508455133942304583236903222948165808559332123348274797826204144723168738177180919299881250404026184124858368.0000000000000000") == true,
        "179769313486231570814527423731704356798070567525844996598917476803157260780028538760589558632766878171540458953514382464234321326889464182768467546703537516986049910576551282076245490090389328944075868508455133942304583236903222948165808559332123348274797826204144723168738177180919299881250404026184124858368.0000000000000000 test case failed");

    M_Assert(
        is_decimal("-.123") == true,
        "-.123 test case failed");

     M_Assert(
        is_decimal("-ABC.123") == false,
        "-ABC.123 test case failed");
     
     M_Assert(
        is_decimal("-123.123.") == false,
        "-123.123. test case failed");

    M_Assert(
        is_decimal("-123..") == false,
        "-123.. test case failed");

    M_Assert(
        is_decimal("--123") == false,
        "--123 test case failed");

    M_Assert(
        is_decimal("-12-3") == false,
        "-12-3 test case failed");

    M_Assert(
        is_decimal(".123") == true,
        ".123 test case failed");

    M_Assert(
        is_decimal("+0.123") == true,
        ".123 test case failed");

    M_Assert(
        is_decimal("+00.123") == true,
        "+00.123 test case failed");

    M_Assert(
        is_decimal("269E-9") == true,
        "269E-9 test case failed");
    
    M_Assert(
        is_decimal("269E9") == true,
        "269E9 test case failed");
    
    M_Assert(
        is_decimal("269e-9") == true,
        "269e-9 test case failed");
}

int main()
{
    auto start = std::chrono::system_clock::now();
    is_decimal_test();
    auto end = std::chrono::system_clock::now();
    std::chrono::duration<double> elapsed_seconds = end - start;
    std::time_t end_time = std::chrono::system_clock::to_time_t(end);
    std::cout << "Computation finished at " << std::ctime(&end_time) << "elapsed time: " << elapsed_seconds.count() << '\n';
    return EXIT_SUCCESS;
}

The output of the test code above:

Computation finished at Mon Jun 10 08:53:38 2024
elapsed time: 6.62e-06

Godbolt link is here.

All suggestions are welcome.

The summary information:

  • Which question it is a follow-up to?

    is_number Function Implementation in C++

  • What changes has been made in the code since last question?

    is_decimal function implementation is presented in this post.

  • Why a new review is being asked for?

    Please review the implementation of is_decimal function and its tests.

\$\endgroup\$
4
  • \$\begingroup\$ Is this an exercise or is there a real case where code needs to test strings and then not convert to a number? Otherwise it would seem less confusing and be more uniform to convert the string to a number (and test its conversion success). \$\endgroup\$ Commented yesterday
  • \$\begingroup\$ A helper with "helper" as part of the identifier is not well-named. Prefer is_decimal_digits(). // We have a nice pair of trivial prefix flags that don't actually peer any further into the "number". Prefer names of is_negative and is_unary_positive. Or combined them as has_sign_prefix. // I'm sad that we don't see a spec conformance promise, e.g. accepting exactly the radix-ten numbers that C++20 accepts. \$\endgroup\$
    – J_H
    Commented yesterday
  • \$\begingroup\$ @J_H However, the name is_decimal_digits() doesn't imply the part of *it == '.', Maybe the name is_decimal_digits_or_radix_character() is better? \$\endgroup\$
    – JimmyHu
    Commented yesterday
  • 1
    \$\begingroup\$ @JimmyHu yeah, I waffled back and forth on that, considering is_digits_with_decimal. Maybe a doc comment needs to expand on the brief identifier, or maybe we really do need a rather verbose identifier. Of the three hard things in CS, naming is the hardest. \$\endgroup\$
    – J_H
    Commented yesterday

3 Answers 3

6
\$\begingroup\$

Tests

Thanks for writing a test suite. This code is well suited to automated testing, and the start you've made here is appreciated.

Since the function we're testing is constexpr and we're providing it with compile-time constant arguments, perhaps we should be using static_assert instead?

However, aborting the tests at the first failure isn't generally desirable. If we break functionality, it's helpful to see which set of tests fails, to get the maximum information to diagnose the cause and the correct fix. I'd prefer to just record the failure (so we can return EXIT_FAILURE) and continue with the next test.

The error printing would be more helpful if the line began with filename and line number in the usual format for log parsers (e.g. with Emacs compilation-mode this enables jump to source line in the editor):

        std::cerr << file << ":" << line << ": Assert failed:\t" << msg << '\n'
                  << "Expected:\t" << expr_str << '\n';

We're missing tests of some edge cases with leading and trailing .. One important test is that of . alone, which should give a false result if we're using the same rules as the C++ language.

Some other missing test inputs (which should all return false): 123.abc, 1e, +, -, +e1, +., 1e+. I'm sure you can add more.

For testing predicates like this, it's often worth creating a collection of the inputs rather than writing each test individually:

bool test_values(bool expected, std::ranges::input_range auto const& values)
{
    auto passed = true;
    for (auto v: values) {
        if (is_decimal(v) == expected) { continue; }
        passed = false;
        std::cerr << std::boolalpha
                  << "is_decimal(\"" << v << "\") returned "
                  << !expected << "; expected " << expected << '\n';
    }
    return passed;
}


bool is_decimal_test()
{
    auto const true_cases = {
        "0", ".0", "0.",
        "01", "09",             // 09 is decimal 9, not octal
        "+0", "-0",
        "1.0", "+.0", "+000.000",
        "2147483648",
        "-2147483649",
        "-340282346638528859811704183484516925440.0000000000000000",
        "+340282346638528859811704183484516925440.0000000000000000",
        "1e0", "+1e-0",
        "1.8e-99",
    };
    auto const false_cases = {
        "", "-", "+", ".", "+.",
        "--0", "-+0", "+-0", "++0",
        "1+", "1-",
        "0..", ".0.", "..0",
        "0x1",
        "e", "1e", "e1",
        "1a0",
        "1e.9",
    };

    return test_values(true, true_cases)
        |  test_values(false, false_cases);
}

I don't see the point in timing the execution of main(). It's just as valid for a user to time the whole program using an external tool (/usr/bin/time or the time built into shells such as Bash). In any case, we shouldn't be using system clock for measuring intervals, but rather a monotonic clock, which isn't subject to adjustments. And we ought to print the units - perhaps by simply streaming the duration rather than .count().


Interface

Functions such as this should be clear which locale is used. In this case, it seems that the global default locale is used for isdigit() but the C locale is used for digit separators (i.e. none) and decimal point(.). It would be better if we could pass a locale object to control which parsing rules are used, but that would obviously inhibit constexpr evaluation. Perhaps we should wholeheartedly jump to the C locale and use our own digit predicate:

    constexpr bool is_digit(char c) noexcept
    {
        return '0' <= c && c <= '9';
    }

We're constrained to parsing only strings of char. The code would be more useful if we could use any string type.

is_decimal_helper() is undocumented and poorly named - perhaps it's not supposed to be part of the public interface. In which case, it should probably live in anonymous namespace.


Implementation

Indentation here is misleading:

return ((is_decimal_helper(s.substr(is_negative_number, s.size())) ||
        is_decimal_helper(s.substr(is_positive_number, s.size()))) &&

That should be

    return ((is_decimal_helper(s.substr(is_negative_number, s.size())) ||
             is_decimal_helper(s.substr(is_positive_number, s.size()))) &&

However, we don't care about the distinction between positive and negative numbers, so we can combine those flags:

    bool has_sign = s.front() == '-' || s.front() == '+';
    return is_decimal_helper(s.substr(has_sign)) && std::ranges::count(s, '.') <= 1
        || is_scientific_notation(s);

I prefer a range-based for over explicit iterator loops when possible. And it might be worth counting decimal points as we go, rather than the caller making a separate pass. That would make is_decimal_helper() clearer to me:

    for (auto decimal_seen = 0u;  unsigned char c: s) {
        if (c == '.') {
            if (decimal_seen++) { return false; }
        } else if (!std::isdigit(c)) {
            return false;
        }
    }
    return true;

In is_scientific_notation() (but not the other functions), we call std::isdigit() with plain char which may be signed. We could avoid that by changing character to unsigned char.

The state machine is clear and well laid out. It could also be simplified using range-based for:

    for (unsigned char character: input) {

I think perhaps that using a state machine is overkill: a scientific-notation decimal is just a simple decimal followed by a (possibly signed) integer. Since a string view has constexpr find(), we could split at e/E to divide and conquer:

    auto pos_e = s.find_first_of("Ee");
    if (pos_e == s.npos) {
        return false;
    }

    return is_simple_decimal(s.substr(0, pos_e))
        && is_integer(s.substr(pos_e+1));

Modified code

Applying the divide-and-conquer approach throughout, I get shorter and arguably clearer code:

#include <algorithm>
#include <ranges>
#include <string_view>

namespace {
    // char-safe C-locale version of std::isdigit()
    constexpr bool is_digit(char c) noexcept
    {
        return '0' <= c && c <= '9';
    }

    constexpr void remove_sign(std::string_view& s) noexcept
    {
        if (!s.empty() && (s.front() == '-' || s.front() == '+')) {
            s.remove_prefix(1);
        }
    }
}

constexpr bool is_integer(std::string_view s)
{
    remove_sign(s);
    return !s.empty()
        && std::ranges::all_of(s, is_digit);
}

constexpr bool is_simple_decimal(std::string_view s) noexcept
{
    remove_sign(s);
    if (auto separator = s.find('.');  separator == s.npos) {
        // no decimal separator
        return !s.empty()
            && std::ranges::all_of(s, is_digit);
    } else {
        return s.length() > 1
            && std::ranges::all_of(s.substr(0, separator), is_digit)
            && std::ranges::all_of(s.substr(separator+1), is_digit);
    }
}

constexpr bool is_scientific_decimal(std::string_view s) noexcept
{
    if (auto pos_e = s.find_first_of("Ee");  pos_e == s.npos) {
        return false;
    } else {
        return is_simple_decimal(s.substr(0, pos_e))
            && is_integer(s.substr(pos_e+1));
    }
}

constexpr bool is_decimal(std::string_view s) noexcept
{
    if (auto pos_e = s.find_first_of("Ee");  pos_e == s.npos) {
        return is_simple_decimal(s);
    } else {
        return is_simple_decimal(s.substr(0, pos_e))
            && is_integer(s.substr(pos_e+1));
    }
}
\$\endgroup\$
5
  • 1
    \$\begingroup\$ Would not '.' always cause a return of false with if (c == '.' && decimal_seen++) { return false; } if (!std::isdigit(c)) { return false; } due to the 2nd if()? \$\endgroup\$ Commented 2 days ago
  • \$\begingroup\$ Some more tests: '1.2e1', '1.23e1', 1.23e2', '1.23e3', '10e-1', and so on. \$\endgroup\$
    – Pablo H
    Commented 2 days ago
  • 1
    \$\begingroup\$ I'd rewrite remove_sign to conditionally call s.removeprefix(1), then unconditionally return s. Internally, that's just a if (...) s._ptr+=1, which is about as efficient as it gets. \$\endgroup\$
    – MSalters
    Commented yesterday
  • \$\begingroup\$ @chux: yes, I think I failed to properly test that fragment. The "Modified code" section is tested, however. \$\endgroup\$ Commented 3 hours ago
  • \$\begingroup\$ @MSalters, I expected that to result in pretty much the same object code. But it turns out to be significantly simpler. That was a surprise! \$\endgroup\$ Commented 3 hours ago
10
\$\begingroup\$

Tear down the state machine

Your use of a state machine is not a bad idea, per se, as a model, but it can be simplified in code.

(If you do insist on using a state machine, then name the states)

Look at the state transitions:

  • From 0: 1, 2, or 3.
  • From 1: 2, or 3.
  • From 2: 2, 4, or 5.
  • From 3: 4.
  • From 4: 4, or 5.
  • From 5: 6.
  • From 6: final.

You will notice that there is no cycle in the state machine. It only ever moves onward. This makes is quite easy to represent as a sequence of instructions, instead.

A sequence of instructions will make it so much more readable!

(There's a whole set of libraries called "Parser Combinators" which allow building parsers out of smaller parsers)

A single instruction can be represented as a named function which removes (or not) characters from the front of the string.

For example, the first one:

//  Pops the leading sign (minus or plus), if any.
//
//  Returns the original if the string doesn't start with a sign.
constexpr auto try_pop_sign(std::string_view s) -> std::string_view {
    if (s.starts_with('+') || s.starts_with('-')) {
        return s.substr(1);
    }

    return s;
}

Notice that the return type is unconditional, there's always a string returned. This is because the sign is optional, so not popping the sign is perfectly fine.

On the other hand, some steps of the algorithm depend on whether a dot or E appears, in which case we need to use a different return type to indicate the possibility that it wasn't found:

//  Pops the leading dot.
//
//  Returns `nullopt` if the string doesn't start with a dot.
constexpr auto pop_dot(std::string_view s) -> std::optional<std::string_view> {
    if (s.starts_with('.')) {
        return std::make_optional(s.substr(1));
    }

    return std::nullopt;
}

From there, you can assemble the entire algorithm in a straightforward manner:

//   WARNING: untested and unreviewed.

//  Pops a number of the form "[+-](\d+|\d+.\d*|.\d+)".
//
//  Returns `nullopt` if the string doesn't match the expected format.
constexpr auto pop_simple_decimal(std::string_view s)
    -> std::optional<std::string_view>
{
    assert(!s.empty());

    s = try_pop_sign(s);

    if (s.empty()) {
        return std::nullopt;
    }

    auto number_digits = s.size();

    s = try_pop_digits(s);

    number_digits -= s.size();

    if (s.empty()) {
        return std::make_optional(s);
    }

    auto fractional = pop_dot(s);

    if (!fractional.has_value()) {
        return std::nullopt;
    }

    s = try_pop_digits(*fractional);

    number_digits += (fractional->size() - s.size());

    if (number_digits > 0) {
        return std::make_optional(s);
    } else {
        return std::nullopt;
    }
}

//  Pops a number of the form "{simple_decimal}([eE]{simple_decimal})?",
//  where "{simple_decimal}" is defined by the `pop_simple_decimal` function.
//
//  Returns `nullopt` if the string doesn't match the expected format.
constexpr auto pop_decimal(std::string_view)
    -> std::optional<std::string>
{
    auto remainder = pop_simple_decimal(s);

    if (!remainder.has_value()) {
        return std::nullopt;
    }

    if (remainder->empty()) {
        return remainder;
    }

    auto exponent = pop_e(*remainder);

    if (!exponent.has_value()) {
        return std::nullopt;
    }

    return pop_simple_decimal(*exponent);
}

//  Checks whether a number matches the "{decimal}" format,
//  as defined by the `pop_decimal` function.
constexpr auto is_decimal(std::string_view s) -> bool {
    return pop_decimal(s).has_value();
}

This is not the most concise, but note how each function has a very linear control flow, making it easy to follow.

\$\endgroup\$
2
  • \$\begingroup\$ Thank you for answering. Where's pop_e function? \$\endgroup\$
    – JimmyHu
    Commented yesterday
  • \$\begingroup\$ @JimmyHu: You'll have to write it, following the models of try_pop_sign and pop_dot I gave. \$\endgroup\$ Commented yesterday
5
\$\begingroup\$

Avoid redundant trips down the string

is_decimal() could be used a lot and deserves some improvements.

return ((is_decimal_helper(s.substr(is_negative_number, s.size())) ||
            is_decimal_helper(s.substr(is_positive_number, s.size()))) &&
            std::ranges::count(s, '.') <= 1) ||
           (is_scientific_notation(s));

s.substr() forms another string just to get rid of a potential leading sign.
Cost O(n) whereas starting the search after the sign could cost O(1).

std::ranges::count(s, '.') walks the entire string yet this walk is already done in is_decimal_helper(). IMO, is_decimal_helper() should also perform the decimal point count.

is_scientific_notation() repeats the task of the preceding code. I'd expect is_decimal_helper() to return some indication, so is_scientific_notation() can pick-up where is_decimal_helper() left off, if possible.

Alternate algorithm:

    digit_found = false;
    dp_found = false;
    
    index = 0;
    if not at string end and s[i] is a sign, i++;
    while (not at string end) {
      if (s[i] == dp) {
        if (dp_found) return false;
        dp_found = true;
      } else if (digit(s[i]) {
        digit_found = true;
      } else {
        break;
      }
      i++;
    }
    if (not digit_found) return false;
    if (s[i] is an 'e' or s[i] is an 'E') {
      digit_found = false;
      if not at string end and s[i] is a sign, i++;
      while (not at string end && digit(s[i]) {
        digit_found = true;
        i++;
      }
      if not at string end or not digit_found, return false;
    }
    return true;
\$\endgroup\$
8
  • 2
    \$\begingroup\$ string_view::substr doesn't return a string, but another view. O(1), and quite sensible here. \$\endgroup\$
    – MSalters
    Commented 2 days ago
  • \$\begingroup\$ @MSalters Agree substr() here is of O(1) and is reasonable. Yet alternative approach only needs an index++. \$\endgroup\$ Commented yesterday
  • 1
    \$\begingroup\$ string_view::substr(1) is also just a single pointer increment. In general, string_view's are quite efficient yet robust. \$\endgroup\$
    – MSalters
    Commented yesterday
  • \$\begingroup\$ @MSalters I'd expect string_view::substr(1) to 1) check if not past end of string (which is redundant given other code), 2) Form a C++ string (with its overhead). Yes efficient in that this is all O(1), yet with excess overhead. 3) IMO, Unless OP is looking for only the most general parsing check (which I doubt given the small range of test values), checking if a string is a valid number might as well instead attempt a numeric conversion along the way. (e.g. strtod() like code), else there risks a subtle difference between this is_decimal() and strtod(). (OP did say decimal) \$\endgroup\$ Commented yesterday
  • 1
    \$\begingroup\$ Whether substr(1) checks for the end of string (defaulted npos argument) is somewhat of an implementation detail, but realistically the optimizer will note that min(size,npos) is just size. You're mistaken with the "form a string", it forms a view. And I don't understand the strtod part. It seems to me you're inserting assumptions there. "Subtle difference"? A rather big difference is that this function does not assume ranges. 1E999999 is a perfectly valid decimal number. \$\endgroup\$
    – MSalters
    Commented yesterday

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.