Why is iostream::eof inside a loop condition (i.e. `while (!stream.eof())`) considered wrong?
Asked Answered
N

5

691

I just found a comment in this answer saying that using iostream::eof in a loop condition is "almost certainly wrong". I generally use something like while(cin>>n) - which I guess implicitly checks for EOF.

Why is checking for eof explicitly using while (!cin.eof()) wrong?

How is it different from using scanf("...",...)!=EOF in C (which I often use with no problems)?

Northward answered 9/4, 2011 at 12:49 Comment(9)
scanf(...) != EOF won't work in C either, because scanf returns the number of fields successfully parsed and assigned. The correct condition is scanf(...) < n where n is the number of fields in the format string.Randolf
@Ben Voigt, it will return a negative number (which EOF usually is defined as such) in case EOF is reachedMakedamakefast
@SebastianGodelet: Actually, it will return EOF if end of file is encountered before the first field conversion (successful or not). If end-of-file is reached between fields, it will return the number of fields succcessfully converted and stored. Which makes comparison to EOF wrong.Randolf
@SebastianGodelet: No, not really. He errs when he says that "past the loop there is no (easy) way to distinguish a proper input from an improper one". In fact it's as easy as checking .eof() after the loop exits.Randolf
@Ben Yes, for this case (reading a simple int). But one can easily come up with a scenario where while(fail) loop terminates with both an actual failure and an eof. Think about if you require 3 ints per iteration (say you are reading an x-y-z point or something), but there is, erroneously, only two ints in the stream.Unrighteous
This issue is analogous to and has the same answer as the C question: Why while(!feof(file)) is always wrong?. Because the flag is set only after hitting the EOF.Welles
Here's C++ FAQ's take on the same question.Welles
@sly: is that "3-ints" scenario not correctly handled by while (in >> x) { if (in >> y >> z) use(x, y, z); else FATAL("got an int not followed by 2 more!"); } if (!eof()) FATAL("didn't get integer where expected");? If not, for what stream content would that not work nicely?Harvest
This question was posted without attribution on Quora: quora.com/unanswered/… (the Quora question title exactly matches the title this question had before it was edited yesterday).Allomorph
C
614

Because iostream::eof will only return true after reading the end of the stream. It does not indicate, that the next read will be the end of the stream.

Consider this (and assume then next read will be at the end of the stream):

while(!inStream.eof()){
  int data;
  // yay, not end of stream yet, now read ...
  inStream >> data;
  // oh crap, now we read the end and *only* now the eof bit will be set (as well as the fail bit)
  // do stuff with (now uninitialized) data
}

Against this:

int data;
while(inStream >> data){
  // when we land here, we can be sure that the read was successful.
  // if it wasn't, the returned stream from operator>> would be converted to false
  // and the loop wouldn't even be entered
  // do stuff with correctly initialized data (hopefully)
}

And on your second question: Because

if(scanf("...",...)!=EOF)

is the same as

if(!(inStream >> data).eof())

and not the same as

if(!inStream.eof())
    inFile >> data
Carthage answered 9/4, 2011 at 12:58 Comment(15)
Worth mentioning is that if (!(inStream >> data).eof()) doesn't do anything useful either. Fallacy 1: It'll not enter the condition if there was no whitespace after the last piece of data (last datum will not be processed). Fallacy 2: It will enter the condition even if reading data failed, as long as EOF was not reached (infinite loop, processing the same old data over and over again).Coign
Slightly off-topic but let me put it. If one uses lazy evaluation, would this approach succeed without any problem?Abreaction
I think it's worth pointing out that this answer is slightly misleading. When extracting ints or std::strings or similar, the EOF bit is set when you extract the one right before the end and the extraction hits the end. You do not need to read again. The reason it doesn't get set when reading from files is because there's an extra \n at the end. I've covered this in another answer. Reading chars is a different matter because it only extracts one at a time and doesn't continue to hit the end.Responser
The main problem is that just because we haven't reached the EOF, doesn't mean the next read will succeed.Responser
@sftrabbit: all true but not very useful... even if there's no trailing '\n' it's reasonable to want other trailing whitespace to be handled consistently with other whitespace throughout the file (i.e. skipped). Further, a subtle consequence of "when you extract the one right before" is that while (!eof()) won't "work" on ints or std::strings when the input is totally empty, so even knowing there's no trailing \n care is needed.Harvest
@TonyD Totally agree. The reason I'm saying it is because I think most people when they read this and similar answers will think that if the stream contains "Hello" (no trailing whitespace or \n) and a std::string is extracted, it will extract the letters from H to o, stop extracting, and then not set the EOF bit. In fact, it would set the EOF bit because it was the EOF that stopped the extraction. Just hoping to clear that up for people.Responser
This answer should be wiki'd, not a day goes when someone doesn't post this antipattern.Dermoid
Also worth mentioning. If there is invalid data on the input the read may cause the stream to go into a bad state. This will prevent further reading (unless explicitly cleared). The test while(!inStream.eof()) will then go into an infinite loop (as no data read and never reaches eof).Vlf
// do stuff with (now uninitialized) data That's no longer true as of C++11, see https://mcmap.net/q/41419/-why-does-stringstream-gt-gt-change-value-of-target-on-failureStrangles
@BaummitAugen It's still left uninitialized if there were no more characters on the stream for example (see my comment on the linked answer). The setting of zero only happens when characters are successfully extracted and then cannot be parsed according to the expected formatPayola
Perhaps I was doing something wrong, but on MSVC, if I do while (std::cin >> word), my program gets stuck in an infinite loop. But it's probably better that I just use std::getline(). convert the input to a string stream then do while (myStringStream >> word), which doesn't get stuck in an infinite loop.Curl
since C++11 one of your comments is wrong/misleading: // do stuff with (now uninitialized) data.Since C++11, if extraction fails then 0 will be written to data. It doesn't alter the point of the question, but readers might get confused when they get 0 and expect "garbage" (not saying that this is a meaningful expectation, but such confusion does happen)Kayekayla
@Someprogrammerdude I am getting infinite loop. As we read EOF it sets eof-bit and fail-bit. Then why on next iteration it don't come out of loop as eof-bit is set. Why then it is going in infinite loop.Mentalism
@AbhishekMane Please create a proper minimal reproducible example and ask as it as a question.Bollay
@Someprogrammerdude stackoverflow.com/q/68780728/11862989 I asked this Q but no answer there. Then user name Evg called this duplicate of https://mcmap.net/q/41420/-c-using-ifstream-with-getline/11862989 I checked there answer it suggest to use fin.clear() so I used it but still having problem of infinite loop. Then user suggests me to read. but I am not understanding this .Mentalism
U
116

Bottom-line top: With proper handling of white space, the following is how eof can be used (and even, be more reliable than fail() for error checking):

while( !(in>>std::ws).eof() ) {
   int data;
   in >> data;
   if ( in.fail() ) /* Handle with 'break' or 'throw' */;
   // Now use data
}

(Thanks Tony D for the suggestion to highlight the answer. See his comment below for an example to why this is more robust.)


The main argument against using eof() seems to be missing an important subtlety about the role of white space. My proposition is that, checking eof() explicitly is not only not "always wrong"—which seems to be an overriding opinion in this and similar Stack Overflow questions—, but with proper handling of white-space, it provides for a cleaner and more reliable error handling, and is the always correct solution (although, not necessarily the tersest).

To summarize what is being suggested as the "proper" termination and read order is the following:

int data;
while(in >> data) {  /* ... */ }

// Which is equivalent to
while( !(in >> data).fail() )  {  /* ... */ }

The failure due to read attempt beyond eof is taken as the termination condition. This means is that there is no easy way to distinguish between a successful stream and one that really fails for reasons other than eof. Take the following streams:

  • 1 2 3 4 5<eof>
  • 1 2 a 3 4 5<eof>
  • a<eof>

while(in>>data) terminates with a set failbit for all three input. In the first and third, eofbit is also set. So past the loop one needs very ugly extra logic to distinguish a proper input (first) from improper ones (second and third).

Whereas, take the following:

while( !in.eof() )
{
   int data;
   in >> data;
   if ( in.fail() ) /* Handle with break or throw */;
   // Now use data
}

Here, in.fail() verifies that as long as there is something to read, it is the correct one. It's purpose is not a mere while loop terminator.

So far so good, but what happens if there is trailing space in the stream—what sounds like the major concern against eof() as terminator?

We don't need to surrender our error handling; just eat up the white-space:

while( !in.eof() )
{
   int data;
   in >> data >> ws; // Eat white space with 'std::ws'
   if ( in.fail() ) /* Handle with 'break' or 'throw' */;
   // Now use data
}

std::ws skips any potential (zero or more) trailing space in the stream while setting the eofbit, and not the failbit. So, in.fail() works as expected, as long as there is at least one data to read. If all-blank streams are also acceptable, then the correct form is:

while( !(in>>ws).eof() )
{
   int data;
   in >> data;
   if ( in.fail() ) /* Handle with 'break' or 'throw' */;
   /* This will never fire if the eof is reached cleanly */
   // Now use data
}

Summary: A properly constructed while(!eof) is not only possible and not wrong, but it allows data to be localized within scope and provides a cleaner separation of error checking from business as usual. That being said, while(!fail) is inarguably a more common and terse idiom, and may be preferred in simple (single data per read type of) scenarios.

Unrighteous answered 23/11, 2012 at 23:30 Comment(5)
"So past the loop there is no (easy) way to distinguish a proper input from an improper one." Except that in one case both eofbit and failbit are set, in the other only failbit is set. You only need to test that once after the loop has terminated, not on every iteration; it will only leave the loop once, so you only need to check why it left the loop once. while (in >> data) works fine for all blank streams.Dicentra
What you are saying (and a point made earlier) is that a bad formatted stream can be identified as !eof & fail past loop. There are cases in which one can not rely on this. See above comment (goo.gl/9mXYX). Eitherway, I am not proposing eof-check as the-always-better alternative. I am merely saying, it is a possible and (in some cases more appropriate) way of doing this, rather than "most certainly wrong!" as it tends to be claimed around here in SO.Unrighteous
"As an example, consider how you'd check for errors where the data is a struct with overloaded operator>> reading multiple fields at once" - a much simpler case supporting your point is stream >> my_int where the stream contains e.g. "-": eofbit and failbit are set. That's worse than the operator>> scenario, where the user-provided overload at least has the option of clearing eofbit before returning to help support while (s >> x) usage. More generally, this answer could use a clean-up - only the final while( !(in>>ws).eof() ) is generally robust, and it's buried at the end.Harvest
With your "3 examples" only the first sets eof. The third does not because the a fails conversion to int and is not read -- it will be left on the input.Derby
so? that's is kind of the point: early break with a bad stream, or end-loop successfully with eof.Unrighteous
D
79

Because if programmers don't write while(stream >> n), they possibly write this:

while(!stream.eof())
{
    stream >> n;
    //some work on n;
}

Here the problem is, you cannot do some work on n without first checking if the stream read was successful, because if it was unsuccessful, your some work on n would produce undesired result.

The whole point is that, eofbit, badbit, or failbit are set after an attempt is made to read from the stream. So if stream >> n fails, then eofbit, badbit, or failbit is set immediately, so its more idiomatic if you write while (stream >> n), because the returned object stream converts to false if there was some failure in reading from the stream and consequently the loop stops. And it converts to true if the read was successful and the loop continues.

Deeann answered 9/4, 2011 at 12:59 Comment(1)
Apart from the mentioned "undesired result" with doing work on the undefined value of n, the program might also fall into an infinite loop, if the failing stream operation doesn't consume any input.Stipulation
E
17

The other answers have explained why the logic is wrong in while (!stream.eof()) and how to fix it. I want to focus on something different:

why is checking for eof explicitly using iostream::eof wrong?

In general terms, checking for eof only is wrong because stream extraction (>>) can fail without hitting the end of the file. If you have e.g. int n; cin >> n; and the stream contains hello, then h is not a valid digit, so extraction will fail without reaching the end of the input.

This issue, combined with the general logic error of checking the stream state before attempting to read from it, which means for N input items the loop will run N+1 times, leads to the following symptoms:

  • If the stream is empty, the loop will run once. >> will fail (there is no input to be read) and all variables that were supposed to be set (by stream >> x) are actually uninitialized. This leads to garbage data being processed, which can manifest as nonsensical results (often huge numbers).

    (If your standard library conforms to C++11, things are a bit different now: A failed >> now sets numeric variables to 0 instead of leaving them uninitialized (except for chars).)

  • If the stream is not empty, the loop will run again after the last valid input. Since in the last iteration all >> operations fail, variables are likely to keep their value from the previous iteration. This can manifest as "the last line is printed twice" or "the last input record is processed twice".

    (This should manifest a bit differently since C++11 (see above): Now you get a "phantom record" of zeroes instead of a repeated last line.)

  • If the stream contains malformed data but you only check for .eof, you end up with an infinite loop. >> will fail to extract any data from the stream, so the loop spins in place without ever reaching the end.


To recap: The solution is to test the success of the >> operation itself, not to use a separate .eof() method: while (stream >> n >> m) { ... }, just as in C you test the success of the scanf call itself: while (scanf("%d%d", &n, &m) == 2) { ... }.

Epicanthus answered 4/5, 2019 at 9:52 Comment(1)
this is the most accurate answer, although as of c++11, i dont believe the variables are uninitialized anymore(the first bullet pt)Salamanca
T
5

The important thing to remember is that inFile.eof() doesn’t become True until after an attempted read fails, because you’ve reached the end of the file. So, in this example, you’ll get an error.

while (!inFile.eof()){
    inFile >> x;
        process(x);
}

The way to make this loop correct is to combine reading and checking into a single operation, like so

while (inFile >> x) 
    process(x); 

By convention, operator>> returns the stream we read from, and a Boolean test on a stream returns False when the stream fails (such as reaching end of file).

So this gives us the correct sequence:

  • read
  • test whether the read succeeds
  • if and only if the test succeeds, process what we’ve read

If you happen to encounter some other problem that prevents you from reading from the file correctly, you will not be able to reach eof() as such. For example, let’s look at something like this

int x; 
while (!inFile.eof()) { 
    inFile >> x; 
    process(x);
} 

Let us trace through the working of the above code, with an example

  • Assume the contents of the file are '1', '2', '3', 'a', 'b'.
  • The loop will read the 1, 2, and 3 correctly.
  • Then it’ll get to a.
  • When it tries to extract a as an int, it’ll fail.
  • The stream is now in a failed state, until or unless we clear the stream, all attempts at reading from it will fail.
  • But, when we test for eof(), it’ll return False, because we’re not at the end of the file, because there’s still a waiting to be read.
  • The loop will keep trying to read from the file, and fail every time, so it never reaches the end of the file.
  • So, the loop above will run forever.

But, if we use a loop like this, we will get the required output.

while (inFile >> x)
    process(x);

In this case, the stream will convert to False not only in case of end of file, but also in case of a failed conversion, such as the a that we can’t read as an integer.

Tanhya answered 19/2, 2022 at 17:21 Comment(1)
?syntax error: undefined identifiers True and FalseFaught

© 2022 - 2024 — McMap. All rights reserved.