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

Parsing a simple tuple "(float,float,float)" in c++11 iostreams

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

Problem

I spotted this tutorial example at compactcpp:

#5 – Parsing strings into “tokens”

#include 
#include 

//This program is extremely crude, but it indeed works for its intended purpose.
//There are many other better ways to do this.
int main() {

    //Here, we use a std::string to hold our input.
    //We will then turn this into a float once we have read in a "full number".
    std::string token;
    char c;

    //Here's how we store our measurements.
    float degrees = 0;
    float minutes = 0;
    float seconds = 0;

    std::cout = '0' && c <= '9' || c == '.') {
            token += c;
        //If we have ended reading a number, let's "cast" to a float
        //then, reset the token std::string
        } else if (c == ',' || c == ')') {
            //All of these if statements translate to:
            //If X is 0 (since 0 == false in C++)
            if (!degrees)
                //We can use the C++11 function called "std::stof",
                //which will turn read our string until it hits a non-number
                //character, and then return it as a float.
                degrees = std::stof(token);
            else if (!minutes)
                minutes = std::stof(token);
            else if (!seconds)
                seconds = std::stof(token);

            //We can use a MEMBER function of a std::string object
            //to clear the std::string for us so that it is empty once again.
            token.clear();
        }

        //As always, we want to halt if we see a ')', which marks the end
        //of our input
        if (c == ')')
            break;
    }
    //This prints out a newline character
    std::cout << "\n";

    //Print out our final variables
    std::cout << "Degrees: " << degrees << '\n';
    std::cout << "Minutes: " << minutes << '\n';
    std::cout << "Seconds: " << seconds << '\n';

    return 0;
}


I thought that was highly unidiomatic C++ code, so I sat down and scribbled down my own version:

Live On Col

Solution

I really like your implementation.

I am especially pleased that I learned something new here:

std::istream wrap(is.rdbuf());
  wrap.imbue(std::locale("C"));


Once you start writing to a stream the locale can not be changed (at least not consistently or in a guaranteed manor). But you get around that problem by using an alternative stream object and re-using the stream buffer. Thus allowing you to use a specific locale (good choice with "C"). But See note below.

Small problem. The stream pushes the locale through to the stream buffer. So you have changed the state of the stream buffer which is not restored.

Normally I would say do:

std::locale  original(wrap.imbue(std::locale("C")); // keep copy of original

...

wrap.imbue(original);  // put the original back.
                       // in both `wrap` and the `stream buffer`


But unfortunately original is the local of the stream wrap not the locale of the stream buffer.

So we need to dig a layer deeper before meddling.

std::locale  original(is.rdbuf().getloc());
std::istream wrap(is.rdbuf());
wrap.imbue(std::locale("C"));

...

is.rdbuf().imbue(original);


Note: That should work. Its been some years since I was digging around in this code. But I seem to remember that once any in/out operations have been started on a stream then an attempt to modify the local was silently ignored. Now this may no longer be true (or it may have been a bug in the implementation I was using at the time or the locale librarys are so under utilized that nobody knows how to use them correctly).

I see what you are doing with expect() there very nice. But it would be even better if we can take that one step further and do this:

// This reads more idiomatic C++ in my opinion
wrap >> Expect('(') >> angle.degrees
     >> Expect(',') >> angle.minutes 
     >> Expect(',') >> angle.seconds
     >> Expect(')');

// But really there is nothing wrong with your way


My output:

struct Expect
{
    char expected;
    Expect(char expected)
        : expected(expected)
    {}
    friend std::istream& operator>>(std::istream& is, Expect const& e)
    {
        char    actual;
        if ((is >> actual) && (actual != e.expected)) {
            is.setstate(std::ios::failbit);
        }

        return is;
    }
};

std::istream& operator>>(std::istream& is, Angle& angle) {
    std::istream wrap(is.rdbuf());
    wrap.imbue(std::locale("C"));

    wrap >> Expect('(') >> angle.degrees
         >> Expect(',') >> angle.minutes
         >> Expect(',') >> angle.seconds
         >> Expect(')');

    is.setstate(wrap.rdstate());
    return is;
}

Code Snippets

std::istream wrap(is.rdbuf());
  wrap.imbue(std::locale("C"));
std::locale  original(wrap.imbue(std::locale("C")); // keep copy of original

...

wrap.imbue(original);  // put the original back.
                       // in both `wrap` and the `stream buffer`
std::locale  original(is.rdbuf().getloc());
std::istream wrap(is.rdbuf());
wrap.imbue(std::locale("C"));

...

is.rdbuf().imbue(original);
// This reads more idiomatic C++ in my opinion
wrap >> Expect('(') >> angle.degrees
     >> Expect(',') >> angle.minutes 
     >> Expect(',') >> angle.seconds
     >> Expect(')');

// But really there is nothing wrong with your way
struct Expect
{
    char expected;
    Expect(char expected)
        : expected(expected)
    {}
    friend std::istream& operator>>(std::istream& is, Expect const& e)
    {
        char    actual;
        if ((is >> actual) && (actual != e.expected)) {
            is.setstate(std::ios::failbit);
        }

        return is;
    }
};

std::istream& operator>>(std::istream& is, Angle& angle) {
    std::istream wrap(is.rdbuf());
    wrap.imbue(std::locale("C"));

    wrap >> Expect('(') >> angle.degrees
         >> Expect(',') >> angle.minutes
         >> Expect(',') >> angle.seconds
         >> Expect(')');

    is.setstate(wrap.rdstate());
    return is;
}

Context

StackExchange Code Review Q#93783, answer score: 3

Revisions (0)

No revisions yet.