HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

A from_string() function (inverse of std::to_string)

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
stdinversefrom_stringfunctionto_string

Problem

I've been thinking of using this:

template 
inline T from_string(const std::string& s){
   std::istringstream iss;
   iss.str(s);
   T result;
   iss >> result;
   return result;
}


in my code. Then I thought "I shouldn't construct istringstreams all the time", and made it into this:

inline std::istringstream& get_istringstream(){
    static thread_local std::istringstream stream;
    stream.str("");
    return stream;
}

template 
inline T from_string(const std::string& s){
   auto& iss(get_istringstream());
   iss.str(s);
   T result;
   iss >> result;
   return result;
}


... and this builds and works (although I haven't tested it very extensively, nor ran performance tests). Would you say that "good enough" for general-purpose utility code, that is not intended to run in some tight loop? Are there other considerations I've overlooked, performance-wise (*) or usability-wise?

Perhaps I should mention my motivation here is partly how I found it strange that there's no std::from_string().

Edit: If you're concerned about the dependence on a default constructor, we can also throw in this:

template
struct istream_traits {
    inline static T read(std::istream& is)
    {
        T x;
        is >> x;
        return x;
    }
};

template<> struct istream_traits {
    inline static bool read(std::istream& is)
    {
        is >> std::boolalpha;
        bool x;
        is >> x;
        return x;
    }
};

template
inline T read(std::istream& is)
{
    T x = istream_traits::read(is);
    return x;
}


... and then replace T result; iss >> result; with return read(iss);.

(*) - Yes, I know anything with iostreams is probably not fast to begin with.

Solution

Just going to review your original code and not the edit.

inline std::istringstream& get_istringstream(){
    static thread_local std::istringstream stream;
    stream.str("");
    return stream;
}

template 
inline T from_string(const std::string& s){
    auto& iss(get_istringstream());
    iss.str(s);
    T result;
    iss >> result;
    return result;
}


As Jerry Coffin mentioned, you are basically reinventing boost::lexical_cast. I wasn't sure if I wanted to review this or flag it as broken code, but it technically compiles and it technically works for one conversion (perhaps a discussion for meta-cr). Test your code thoroughly.


Are there other considerations I've overlooked, performance-wise (*) or usability-wise?

You don't handle errors. Consider someone doing this:

auto x1 = from_string("Not an int");


What is the value of x1?

T result;      // uninitialized variable.
    iss >> result; // operator>> fails silently, sets failbit
    return result; // return uninitialized variable.


The result is undefined behavior since we are returning an uninitialized value that was never assigned to. Remember to check that the reading of a value was successful. Notify the callee whenever that an error, like invalid argument or out of range, has occurred.

There are different error handling strategies today. The common ones are exceptions, std::system_error, and Alexandrescu's expected (currently being proposed for standardization).

Let's say someone does something common, like use your function more than once.

auto x1 = from_string("42");
auto x2 = from_string("43");


The first line returns 42, that is fine. The second line has an issue.

T result;      // uninitialized variable.
    iss >> result; // eofbit enabled from previous stream consumption
    return result; // return uninitialized variable.


Undefined behavior again. The original code had

std::istringstream iss;


On construction, isss internals were initialized (data and error state). In your refactored code, you wanted to mimic a newly constructed object

inline std::istringstream& get_istringstream(){
    static thread_local std::istringstream stream;
    stream.str("");
    return stream;
}

    auto& iss(get_istringstream());


Turns out your refactored code wasn't equivalent. You forgot to clear the error state.

Consider the following code:

auto x1 = from_string("42,004");


What is the value of x1? Conversion of arithmetic types using separators is dependent on the locale of the stream. The possible results are

  • forty-two thousand and four,



  • forty two and four one-thousandths, or



  • forty-two.



If you are fine with this behaviour, document it. If you want to ensure you always use the behavior of a specific locale by default, consider imbuing before reading.


Perhaps I should mention my motivation here is partly how I found it strange that there's no std::from_string().

` provides functions to convert to arithmetic types

  • Reals - std::stod, std::stold, std::stof



  • Signed integrals - std::stoi, std::stol, std::stoll, and



  • Unsigned integrals std::stoul, std::stoull.



You could also consider copying the naming scheme from these and have
string_to.

For general purpose conversions, the ones that immediately come to mind are
boost::lexical_cast (stream-based) and boost::spirit (policy-based). Boost also has boost::convert, an adapter library built to work with various conversion utilities (strtol, boost::spirit, boost::lexical_cast`, and C++-Streams).

Code Snippets

inline std::istringstream& get_istringstream(){
    static thread_local std::istringstream stream;
    stream.str("");
    return stream;
}

template <typename T>
inline T from_string(const std::string& s){
    auto& iss(get_istringstream());
    iss.str(s);
    T result;
    iss >> result;
    return result;
}
auto x1 = from_string<int>("Not an int");
T result;      // uninitialized variable.
    iss >> result; // operator>> fails silently, sets failbit
    return result; // return uninitialized variable.
auto x1 = from_string<int>("42");
auto x2 = from_string<int>("43");
T result;      // uninitialized variable.
    iss >> result; // eofbit enabled from previous stream consumption
    return result; // return uninitialized variable.

Context

StackExchange Code Review Q#139702, answer score: 5

Revisions (0)

No revisions yet.