0

I was reading the following code sample:

std::array<int, 4> parseMessage(const std::string& input) {
    std::stringstream ss(input);
    std::array<int, 4> message;
    int n;
    // Loop over all characters in the string and ignore the semicolons.
    for (int i = 0; ss >> n && i < 4; ++i) {
        message[i] = n;
        if (ss.peek() == ';') {
            ss.ignore();
        }
    }
    return message;
}

Can someone explain why someone would do ss>>n in the loop condition area ? It looks a bit odd to me.

Rahul Iyer
  • 17,034
  • 15
  • 76
  • 159
  • 2
    There is an issue with this function: If the message has less than 4 numbers, this function will return uninitialized values with no way for the caller to know. – interjay Oct 11 '19 at 06:58
  • 1
    Using that as a condition is the most common "read until you can't read any more" method. The most common mistake is [using `!stream.eof()`](https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong) – molbdnilo Oct 11 '19 at 07:17
  • 1
    It would be better to return a `std::vector` so the user could check `size()` afterwards since the extraction may come up short. The condition would probably be better written as `i < 4 && ss >> n`. There's no need to extract any 5:th value since you'll only throw it away. – Ted Lyngmo Oct 11 '19 at 07:30

2 Answers2

4

It will first read a an int from the stream and then evaluate if the stream is good. (because s >> n will return a reference to s).

Evaluating a stream as bool:

Returns true if the stream has no errors and is ready for I/O operations. Specifically, returns !fail().

So as soon as the stream fails to read an int or reaches the end of stream (e.g. end of input) it will evaluate as false and end the loop.

So this code will extract up to 4 ints from a given input (4, because of the && i < 4).

churill
  • 9,299
  • 3
  • 13
  • 23
1

>> reads into the variable on the right hand side of the expression and returns the stream.

Using a stream in a boolean expression returns whether the stream has not failed.

Your for loop is therefore equivalent to:

int i = 0;
while ( i < 4 )
{
  ss >> n;
  if ( ss.fail() )
  {
    break;
  }
  message[i] = n;
  if (ss.peek() == ';') {
     ss.ignore();
  }
  i++;
}

It reads up to 4 numbers and stops if the contents of the stream are not convertible to a number.

Alan Birtles
  • 22,711
  • 4
  • 22
  • 44
  • What is your opinion of the readability of the function as is ? And do you see any problems with it ? – Rahul Iyer Oct 11 '19 at 07:06
  • Depends who is reading it, if you can't understand it and you find my version more readable then the more verbose version is better. An experienced c++ developer shouldn't have any difficulty understanding your version. In professional programming you should always consider when writing code whether someone else (or even you in the future) will be able to understand and maintain your code, so keeping it simple is better than using weird tricks to make the code shorter/(theoretically) faster – Alan Birtles Oct 11 '19 at 07:10