2
\$\begingroup\$

You might consider this question a follow-up of:

BigInteger check in C from a string

Although, this time, I used C++.

I am on Linux Mint 19.1 with the compiler version:

g++-8 (Ubuntu 8.2.0-1ubuntu2~18.04) 8.2.0

I tried to push myself hard in C, but it obviously requires one of:

  • more skill / experience

  • lower-level approach

or both.

That is why I decided to transition to C++, already bought one C++ online course on Udemy, but back to coding, the current version looks straightforward to me, I see no more pointers, etc., which sounds ideal to me:


#include <iostream>
//#include <string> // I don't get why it compiles without this header

bool string_contains_integer(std::string str)
/*
    This function iterates through an array of chars,
    and checks each character to be a digit;
    optionally including a starting "+/-" sign.

    Returns true if the string contains a number (string of digits);
    Returns false if the string contains another character(s).

    Starting "+/-" gets ignored, as we accept all integer numbers.
*/
{
    // if the string is empty, return right away
    if ( str.empty() ) return false;

    // I'd like to avoid repeated length() calls
    unsigned long long string_length = str.length();

    // find out if there is a "+/-" sign
    bool sign_present = ( (str[0] == '-') || (str[0] == '+') );

    // check if there is the sign only
    if ( sign_present && (string_length == 1) ) return false;

    // iterate through all characters in the string
    for (unsigned long long i = 0; i < string_length; i++)
    {
        // skip the digit check for the sign at the beginning
        if ( (i == 0) && sign_present ) continue;

        // this is actually the core part checking on digits
        if ( ! std::isdigit( (unsigned char) str[i] ) ) return false;
    }

    // If we got here, then all the characters are digits,
    // possibly starting with a sign.
    return true;
}

int main(void)
{
    if ( string_contains_integer("-123456789123456789123456789123456789123456789123456789123456789") )
    {
        std::cout << "PASS: Input is a number.\n";
        return 0;
    }
    else
    {
        std::cerr << "FAIL: Input is not a number!\n";
        return 1;
    }
}

This program I compile as follows:

g++-8 -std=c++17 -Wall -Wextra -Werror -Wpedantic -pedantic-errors -o bigInteger bigInteger.cpp
\$\endgroup\$
5
\$\begingroup\$

You do need to include <string>. Your platform seems to bring it in as a side-effect of other includes, but you can't portably rely on that.

If there's no need to modify the contents of the string, prefer to pass by reference, to reduce copying:

bool string_contains_integer(const std::string& str)
//                           ^^^^^            ^

Instead of looping with indexes, learn to use iterators. If you really must use indexes, use the correct type (std::string::size_type, not unsigned long long).

We don't need to write our own loop by hand, as there's a standard algorithm that will do that for us; we just need to supply the correct predicate function:

#include <algorithm>
#include <iostream>
#include <string>

bool string_contains_integer(const std::string& str)
/*
    This function iterates through an array of chars,
    and checks each character to be a digit;
    optionally including a starting "+/-" sign.

    Returns true if the string contains a number (string of digits);
    Returns false if the string contains any other character(s).

    Starting "+/-" gets ignored, as we accept all integer numbers.
*/
{
    auto from = str.begin();
    auto to = str.end();

    if (from == to) {
        return false;
    }

    if (*from == '+' || *from == '-') {
        ++from;
    }

    return from != to
        && std::all_of(from, to,
                       [](unsigned char c){ return std::isdigit(c); });
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Or better yet, use a std::string_view. \$\endgroup\$ – Deduplicator May 6 at 1:24
2
\$\begingroup\$

In this self-mini-review, let it be clear, I just want to point out several things I do differently now. These were untouched by Toby Speight, so could be beneficial to someone. All of these are opinion-based!

Styling points

  • I add spaces around parentheses now, like in this case:

    bool string_contains_integer ( const std::string & str )
    
  • I write clearer function's function comments:

    // True : If the string contains an integer number (possibly starting with a sign).
    // False: If the string contains some other character(s).
    
  • I no longer use redundant parentheses, like in this case:

    if ( i == 0 && sign_present ) continue;
    
  • I put the function's function comment on top of the function:

    // True : If the string contains an integer number (possibly starting with a sign).
    // False: If the string contains some other character(s).
    bool string_contains_integer ( const std::string & str )
    

    because I found out there is a helper in Visual Studio Code - if If I hover the mouse over the function call anywhere in the code:

    Helper in Visual Studio Code


Compilation points

I use -Wc++11-compat flag to ensure compatibility with old compilers.

Warn about C++ constructs whose meaning differs between ISO C++ 1998 and ISO C++ 2011, e.g., identifiers in ISO C++ 1998 that are keywords in ISO C++ 2011. This warning turns on -Wnarrowing and is enabled by -Wall.

Even since I use -Wall already, it could be used explicitly in case you for any reason opt to remove -Wall. The compatibility flag could be useful.


Code points

I no longer call str.empty() as this was basically a redundant call of str.length():

std::size_t str_length = str.length();
if ( str_length == 0 ) return false;

Editor points

Further, I completely switched to Visual Studio Code, where it looks just great for a reader:

string_contains_integer() in Visual Studio Code

\$\endgroup\$
  • 1
    \$\begingroup\$ Personally, I don't like the whitespace after ( and function names, or before ). But that's merely a preference, and it's good that you have a consistent style. Many of us would recommend putting the body statement of if, for etc. on a new line, and many of us recommend using braces even for a single statement. \$\endgroup\$ – Toby Speight May 7 at 17:37
  • \$\begingroup\$ @TobySpeight Thank you for your kind insights. Let me try it and see what fits me best. \$\endgroup\$ – LinuxSecurityFreak May 7 at 19:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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