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

Question

I spotted this tutorial example at compactcpp:

#5 – Parsing strings into “tokens”

#include <iostream>
#include <string>

//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 << "Please input a degrees, minutes, seconds measurement as (D,M,S): ";

    //While we read in characters
    while(std::cin.get(c)) {
        //If it is part of a number, it is part of our "token"
        if (c >= '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 Coliru

#include <sstream>
#include <iostream>
#include <iomanip>
#include <limits>

namespace mylib
{
    struct Angle {
        double degrees = 0, minutes = 0, seconds = 0;
    };

    std::ostream& operator<<(std::ostream& os, Angle const& angle) {
        std::ostream wrap(os.rdbuf());
        wrap.imbue(std::locale("C"));

        wrap << std::fixed << std::setprecision(std::numeric_limits<double>::digits10) 
            << '(' 
            << angle.degrees << ", " 
            << angle.minutes << ", " 
            << angle.seconds 
            << ')';
        os.setstate(wrap.rdstate());
        return os;
    }

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

        auto expect = [&](char const expected) { 
            char actual; 

            if (!(wrap >> actual))
                return false;       // state already bad()

            if (actual != expected) // force fail
                wrap.setstate(wrap.rdstate() | std::ios::failbit);

            return actual == expected; 
        };

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

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

int main() {
    std::istringstream iss("(1.24, 9001, 0.000000134)");

    mylib::Angle parsed;

    if (iss >> parsed)
        std::cout << "Parsed: " << parsed << "\n";
    else
        std::cout << "Parsing did not succeed\n";
}

What do you think can be improved here to make it more elegant and robust?

Answer 1

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;
}

Answer 2

To be frank: Your implementation does not target the topic token scanning. It is targeted at serialization / deserialization.

But the example that it is targeted at is no example of token scanning neither. Tokens in the example would be the '.', [0-9]+, token scanning would first build a token tree made of this tokens before interpreting the tokens in a grammatical context. The implementation you choose is very clear. For the example used it could be irritating to the reader as it introduces other non-trivial concepts but I personally tend to teach clear and structured programming over examples that lead to bad code design (and mark it as "is extremely crude").

I personally would not recommend recursion in operator implementations though, especially not in stream operators. You would not want to call a stream operator and get an error such as "too many recursions".