Converting std::string to int without Boost

Question

I'm trying to convert an std::string to an int with a default value if the conversion fails. C's atoi() is way too forgiving and I've found that boost::lexical_cast is quite slow in the case where the cast fails. I imagine it's because an exception is raised. I don't have C++11, so stoi() is out.

The Delphi function StrToIntDef is the ideal, and it's also available in C-Builder (where I'm working currently). But I want something more portable that works with std::string.

I've created the following function, which uses atoi() but first tests for error conditions. It also allows for leading and trailing spaces, which are harmless in my situation.

int stringtoIntDef(const std::string & sValue, const int & DefaultValue) {
// convert a std::string to integer with a default value returned
// in the case where the string doesn't represent a valid integer 
// - accepts leading or trailing spaces as valid

   bool hasDigits = false;
   bool TrailingSpace = false;
   for (std::string::size_type k = 0; k < sValue.size(); ++k) {
      if ((sValue[k] == ' ') || (sValue[k] == '\t')) {
         TrailingSpace = hasDigits;
      } else if ((sValue[k] == '0') || (sValue[k] == '1') || (sValue[k] == '2') || (sValue[k] == '3') ||
                  (sValue[k] == '4') || (sValue[k] == '5') || (sValue[k] == '6') || (sValue[k] == '7') ||
                  (sValue[k] == '8') || (sValue[k] == '9')) {
         if (TrailingSpace) {
            return DefaultValue;
         } else {
            hasDigits = true;
         }
      } else if ((sValue[k] == '-') && !hasDigits) {
         hasDigits = true; // this protects against "--"
      } else {
         return DefaultValue;
      }
   }
   return atoi(sValue.c_str());
}

In my testing, I've compared it against StrToIntDef and it is just as fast, but lexical_cast is much slower in the case where the default is returned. For 1000 iterations, lexical_cast took 5 seconds while the other 2 weren't measurable.

For my lexical_cast function, I've used this:

try {
    return boost::lexical_cast<int>(sValue);
}
catch (boost::bad_lexical_cast &) {
    return DefaultValue;
}

Are there any gotchas I might have missed here?

Answer 1

This is a follow up to your second follow up. I just have a few comments, and you have two bugs.

• stringtoIntDef is a strange name. In particular, why are Int and Def capitalized but not To? I would go either full camelCase, or I would use the standard library convention of under_scores.
• You never delete[] s which means you have a memory leak.
• I actually meant in my comment that you could check the end pointer from strtol to see if all that follows is either nothing or 0. I should have been more specific. An example of this is below.
• If cstdlib was included (as it should be) rather than stdlib.h, then strtol needs to be std::strtol. The cheader-style headers guarantee declarations in the std namespace, but they do not guarantee declarations in the global namespace. (Every C++ implementation I've ever seen does both, but they not required to do the global namespace.)
• Define variables as late as possible. In other words, declare result when you're defining it to be strtol's return value.
• If you don't want a variable to change after its initial value, make it const. This keeps you from accidentally changing the value, and it makes your intentions more clear to anyone who inherits the code.
• You must provide a base of 10 to strtol, not 0. 0 is the default and it means to try to parse a number as base 10 unless it's prefixed with either 0x (for hex) or 0 (for octal). Assuming you want just decimal, it must be 10.
• If you do want the default behavior, don't put the 0. Just let it default to 0. Default behavior and and an empty default value have a nice symmetry to them, and it allows for future change of what the default sentinel value is without breaking consuming code.
• Since it's never modified anywhere, str should be passed by constant reference, not reference.
• If you were going to copy the string (don't! memory allocation is both unnecessary here and is very slow compared to the rest of the operations we're using), I would take the string by value and modify the parameter. That would allow for moving in C++11, and it would conveniently handle the memory allocation for you. The (potentially major) downside to this approach would be that obviously invalid strings could cause a rather large copy. Imagine if you passed a string of pure whitespace. All that whitespace would be copied for nothing other than to get immediately removed.
• We both missed the case of an empty string. strtol output can be used to detect it (begp == endp), but neither of us checked for that :).

---

I still disagree with your decision to hide the ability to check for errors, but if that's the route you want to take, I would do something like this:

int string_to_int(const std::string& str, const int& DefaultValue) {
    errno = 0;

    const char* str_beg = str.c_str();
    const char* str_end = str_beg + str.size();
    char* p = NULL;

    const int result = std::strtol(str.c_str(), &p, 10);

    if (p == str_beg) { return DefaultValue; }

    for (; p != str_end && std::isspace(*p); ++p) { /* eat trailing spaces */}

    if (p != str_end || errno != 0) {
        return DefaultValue;
    }

    return result;
}

(This could probably be optimized quite a bit, but it would likely be compiler/system specific, and it would be tedious.)

Answer 2

There's a few things that jump out at me both in your implementation and standard options you missed. A few (scattered) thoughts follow.

---

Returning a default if you care about error checking doesn't make sense. How would you determine if conversion failed if there's not an integer available for use as a sentinel? You are basically doing the same thing atoi does except your default is flexible instead of 0.

---

Have you considered [strtol][1]? It's basically atoi with error checking capabilities. It's the fastest string to int converter you're likely going to get:

long string_to_long(const std::string& str, long default_value, int base = 10) {
    char* parse_end = NULL; //nullptr if C++11, though really you don't have to initialize this
                            //since strol is guaranteed to initialize it. I just like to avoid
                            //unitialized variables.
    long val = strtol(str.c_str(), &parse_end, base);
    if (parse_end == str.c_str() + str.size()) {
        return val;
    } else {
        return default_value;
    }
}

Note: strtol ignores leading whitespace. The function I've written above errors for trailing whitespace (or trailing anything). In other words, " 3" is fine, but "3 " would error.

---

The idiomatic way to do this in C++ would be to use a string stream:

int string_to_int(const std::string& str, int def)
{
    std::istringstream ss(str);
    int i;
    if (!(ss >> std::noskipws >> i)) {
        //Extracting an int failed
        return def;
    }
    char c;
    if (ss >> c) {
        //There was something after the int
        return def;
    }
    return i;
}

In terms of performance, strtol will likely be faster than this. This does have a useful advantage though that it can be made highly generic with simple templating (this is the essential idea behind boost::lexical_cast actually, but this doesn't use exceptions):

template<typename ResultType>
ResultType lexical_cast(const std::string& str, const ResultType& default_value = ResultType()) {
    std::istringstream ss(str);
    ResultType result;
    if (!(ss >> std::noskipws >> result)) {
        return default_value;
    }
    char c;
    if (ss >> c) {
        return default_value;
    }
    return result;
}

---

If you don't want to use exceptions, but you want to be able to detect errors, you're going to have to use a flag. You basically have two options of how to handle that. You can use a boolean flag and return the value, or you can use a boolean reference parameter and return the value. Which route you take really depends on how you want to use the code, but I would probably return the value. That lets you ignore errors if you want to, but it still doesn't force you to. I might do something like this:

template<typename ResultType>
ResultType lexical_cast(const std::string& str, bool& success) {
    std::istringstream ss(str);
    ResultType result;
    success = true;
    if (!(ss >> std::noskipws >> result)) {
        success = false;
    }
    char c;
    if (ss >> c) {
        success = false;
    }
    return result;
}

template<typename ResultType>
ResultType lexical_cast(const std::string& str) {
    bool ignore;
    return lexical_cast<ResultType>(str, ignore);
}

This gives you both options of detecting an error or ignoring one. You could even build more on top of this to have one that returns a default value:

template<typename ResultType>
ResultType lexical_cast(const std::string& str, const ResultType& default_value = ResultType()) {
    bool success;
    ResultType val(lexical_cast<ResultType>(str, ignore));
    if (success) {
        return val;
    } else {
        return default_value;
    }
}

---

Don't check for classes of characters manually. Just use the cctype header's functions (isspace, isdigit, etc).

---

You loop is way over complicated:

bool all_digit = true;
for (std::string::size_type i = 0, l = str.size(); i < l; ++i) {
    if (!std::isdigit(str[i])) { 
       all_digit = false;
        break;
    }
}

if (all_digit) { ... }

Or, allowing leading/trailing whitespace:

std::string::size_t i = 0;
const std::string::size_t len = str.size();
for (; i < len && std::isspace(str[i]); ++i) { /* empty */ }
for (; i < len && std::isdigit(str[i]); ++i) { /* empty */ }
for (; i < len && std::isspace(str[i]); ++i) { /* empty */ }
const bool all_digits = (i == len);

Answer 3

As per @retailcoder's suggestion I'm adding the bulletproof version as a separate answer rather than edit @Corbin's answer.

This function meets all the requirements and is quite a bit cleaner than my original. It is based on @Corbin's final suggestions.

int stringtoIntDef(const std::string & str, const int & DefaultValue) {
   int result = DefaultValue;
   std::size_t i = 0;
   const std::size_t len = str.size();
   for (; i < len && std::isspace(str[i]); ++i) { /* leading space */}
   if (str[i]=='-') ++i; /* allow negative symbol */
   if (i==len) { return result; } /* no digits */
   for (; i < len && std::isdigit(str[i]); ++i) { /* digits */}
   for (; i < len && std::isspace(str[i]); ++i) { /* trailing space */}
   if (i == len) {
      errno = 0;
      const char *s = str.c_str();
      char * p;
      result = strtol(s, &p, 0);
      if (errno != 0) {
         result = DefaultValue;
      }
   }
   return result;
}

Interestingly it remains a fair bit slower than the Delphi function. Some sample results using GetTickCount(), which has a granularity of about 16, and 1 million iterations:

             str  time   ok  str time  ok   str time ok   str time ok   str time ok    str time ok
Delphi       -25    0   yes  12    16  yes  box  15  yes  " "  15  yes  inf  16  yes   "2 "  16  no
My original  -25   140  yes  12   109  yes  box  31  yes  " "  109  no  inf 375  no    "2 "  78  yes
Final        -25   250  yes  12   234  yes  box  93  yes  " "  63  yes  inf  561 yes   "2 "  249 yes

where inf is some number large enough to cause overflow.

Admittedly none of those times is very big for 1 million of anything. I just think its interesting that the Delphi function is SO efficient.

Answer 4

Based on @Corbin's last comment I've created another version that seems to be a bit faster and cleaner. I just strip any whitespace off the end before calling strtol. It also has the benefit of handling numbers with a different base, if I want it to.

int stringtoIntDef(const std::string & str, const int & DefaultValue) {
   // this function was developed iteratively on Codereview.stackexchange
   // with the assistance of @Corbin
   std::size_t len = str.size();
   while (std::isspace(str[len-1]))
       len--;
   if (len==0) return DefaultValue;    
   errno = 0;
   char *s = new char[len + 1];
   std::strncpy(s, str.c_str(), len);
   char * p;
   int result = strtol(s, &p, 0);
   if ((*p!= '\0') || (errno != 0)) {
      return DefaultValue;
   }
   delete s;
   return result;
}