-2

I am writing a program to take input from a file and display it on the console. The problem is that the last entry is being repeated twice. The code is as follows:-

int main(void)
{
    string filename;
    int grades;
    cout<<"Enter the filename:"<<endl;
    getline(cin,filename);
    ifstream inFile(filename.c_str(),ios::in);
    if(!inFile){
        cout<<"File does not exist."<<endl;
        exit(1);
    }
    while(!inFile.eof())
    {
        inFile>>grades;
        cout<<grades<<endl;
    }
    inFile.close();
    return 0;
}

Can you please help me in finding out the mistake? I have searched on the web and my code seems to be syntactically and logically correct.

kusur
  • 538
  • 10
  • 31
  • dozens of post already on SO, please do a little search – P0W Sep 03 '13 at 12:35
  • `while(inFile>>grades)` – BoBTFish Sep 03 '13 at 12:38
  • Once more: **Don't use `while(!inFile.eof())`. Don't use `while(inFile.good())` either.** See [Why is iostream::eof inside a loop condition considered wrong?](http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong) – gx_ Sep 03 '13 at 12:45
  • The real question is where he got this idiom from. Any site or book which uses it is good for the garbage can, and nothing else. – James Kanze Sep 03 '13 at 13:40

2 Answers2

2

This is wrong

while(!inFile.eof())
    {
        inFile>>grades;
        cout<<grades<<endl;
    }

This is right

while (inFile >> grades)
{
   cout << grades << endl;
}

Must be the single most common error on this forum. eof() does not tell you that the next read will have an end of file error, it tells you that the last read failed because of end of file. So if you must use eof() you should use it after you read not before.

john
  • 71,156
  • 4
  • 49
  • 68
  • It doesn't tell you whether the last read has failed. Whether `eof()` returns true or not after a successful read depends on what you were reading, and where you were in the input sequence. All you can say is that _if_ `eof()` returns true, the next read will fail (but if it returns false, the next read might fail as well). And that _if_ `fail()` returns true, and `eof()` is true, there's a good chance (but not 100%) that the reason for failure was that we'd reached the end of file before reading the value. – James Kanze Sep 03 '13 at 13:28
  • And concerning your last sentence: there is _no_ reasonable use of `eof()` before you've detected failure by other means. It's only use is to try to determine the reason for a failure, i.e. `fail() && !eof()` means that you had a format error in the input. – James Kanze Sep 03 '13 at 13:30
0

Syntactically correct, yes. But not logically. You're using eof() incorrectly.

The first thing to realize is that all of the functions which test status base their results on the last input. And you must always check that the input succeeded before using anything you've input; when you write:

inFile >> grades;
std::cout << grades;

you are not verifying that the input succeeded before accessing grades. In this case, if the input fails, you get the previous value; if there were no previous value, you get undefined behavior. Somewhere between the >> and the <<, you must check that the >> succeeded.

The usual way of checking for success is to use the stream itself as a boolean. And because the >> returns a reference to the stream, the idiomatic way of writing your loop would be:

while ( inFile >> grades ) {
    std::cout << grades << std::endl;
}

From a software engineering point of view, it's horrible (modifying state in the condition of a while), but the idiom is so ubiquitous that anything else raises questions.

This will stop if there is an input error for any reason. Once you've seen the failure (and only then), you can ask why:

if ( inFile.bad() ) {
    //  Serious hardware failure...
} else if ( !inFile.eof() ) {
    //  Format error in the input...
} else {
    //  Normally, no more data in the input stream, but
    //  there are a few special cases where you could still
    //  have a format error and end up here.  Not with
    //  `int`, however.
}

But again, this is only valid after an input has failed.

James Kanze
  • 142,482
  • 15
  • 169
  • 310