17

Possible Duplicate:
Why is iostream::eof inside a loop condition considered wrong?

I have the following piece of code:

ifstream f("x.txt");
string line;
while (f.good()) {
  getline(f, line);
  // Use line here.
}

But this reads the last line twice. Why does this happen and how do I fix it?

Something very similar happens with:

ifstream f("x.txt");
string line;
while (!f.eof()) {
  getline(f, line);
  // Use line here.
}
Community
  • 1
  • 1
Prabhu
  • 2,960
  • 7
  • 33
  • 45
  • 4
    How is this a duplicate? The other answer doesn't even mention looping with the good() function as the test. – Jerry Jeremiah May 15 '14 at 21:51
  • 1
    @JerryJeremiah It's the same general issue of trying inspect the current stream state (e.g. `.good()`, `.eof()`, `.fail()`) in order to predict the success of future read operations (e.g. `getline`, `>>`). This does not work because the stream state tells you whether a previous read operation failed / reached the end of the input; it does not tell you anything about future read attempts. – melpomene May 04 '19 at 10:09

3 Answers3

33

You very, very rarely want to check bad, eof, and good. In particular for eof (as !stream.eof() is a common mistake), the stream currently being at EOF does not necessarily mean the last input operation failed; conversely, not being at EOF does not mean the last input was successful.

All of the stream state functions – fail, bad, eof, and good – tell you the current state of the stream rather than predicting the success of a future operation. Check the stream itself (which is equivalent to an inverted fail check) after the desired operation:

if (getline(stream, line)) {
  use(line);
}
else {
  handle_error();
}

if (stream >> foo >> bar) {
  use(foo, bar);
}
else {
  handle_error();
}

if (!(stream >> foo)) {  // operator! is overloaded for streams
  throw SomeException();
}
use(foo);

To read and process all lines:

for (std::string line; getline(stream, line);) {
  process(line);
}

Pointedly, good() is misnamed and is not equivalent to testing the stream itself (which the above examples do).

Fred Nurk
  • 13,035
  • 4
  • 33
  • 61
  • The part about checking eof is correct, but the suggestion to check the stream itself is a bit off. `good()` means none of eofbit, badbit, or failbit are set. `fail()` means either badbit or failbit is set. Checking the stream (either using the void* converstion, or operator !) is exactly the same as calling the fail() member function. – KeithB Dec 01 '10 at 15:19
  • 2
    @KeithB: You might notice I left fail out of the "should be rarely checked" group. A failed stream is what's important, and checking the stream itself is almost always more convenient than the equivalent fail(). Compare getline(stream, line) to !getline(stream, line).fail(). – Fred Nurk Dec 01 '10 at 15:46
9

Just use

ifstream f("x.txt");
while (getline(f, line)) {
    // whatever
}

This is the idiomatic way to write such a loop. I've not been able to reproduce the error (on a Linux machine).

Fred Foo
  • 328,932
  • 68
  • 689
  • 800
  • I've only just realised why that works: The last successful call to getline() _might_ set **eof**, if the last line does not have a newline at the end. The **fail** bit is set only when there is an unsuccessful call to getline(). So we _don't_ want to end the loop at **eof**, but we do want to end it at **fail**. – Aaron McDaid May 19 '11 at 23:27
  • One more thing.. I had been doing `while(f.peek() != EOF) { ... }`. I think this is correct? But I'll use your answer in future. – Aaron McDaid May 19 '11 at 23:34
2

It didn't read the last line twice but because it failed to read when it reached eof, your string line has the value it had previously.

That is because f is no longer "good" when it has read EOF, not when it is about to read it.

CashCow
  • 29,087
  • 4
  • 53
  • 86