More about EOF in loop condition

Question

Okay, when I saw this thread: Why is iostream::eof inside a loop condition (i.e. `while (!stream.eof())`) considered wrong?

I read the answers but I really didn't understand what is wrong in this, maybe because I don't have much experience in c++ but my code works exactly the way it's supposed to.

int main()
{
    ifstream file;
    string text, search;
    int offset;
    cout << "Enter a word: "; cin >> search;
    file.open("Find.txt");
    if (file.is_open()) {
        while (!file.eof()) {
            file >> text;
            offset = text.find(search, 0);
            if (offset != string::npos) {
                cout << text << endl;
            }
        }
    }
    else {
        cout << "Error!";
        return 0;
    }
    file.close();
}

I enter in a word, and it searches for it in a text file, and I had zero problems using that. So, when is this condition considered wrong?

Because file >> text; can fail and you never know it because you never check it. You would thus march on and use whatever crap happens to be in text, potentially the last time is succeeded.Did you try searching for the last word in your file? — WhozCraig

Lightness Races in Orbit Lightness Races in Orbit · Accepted Answer · 2019-06-13T10:12:46

I won't repeat everything said in that thread; that would be a waste of time.

If you did this:

    while (!file.eof()) {
        file >> text;
        // offset = text.find(search, 0);
        // if (offset != string::npos) {
            cout << "Text: " << text << endl;
        // }
    }

… then you would see the problem: an "extra" loop iteration every single time you run the program, with text having whatever value it had for the previous iteration.

Since your output is conditional on the result of searching in text for a specific substring, if that search on the final line of input will fail, you get away with it! However, if the last line matches, you'll again see the problem.

This manner of loop-input does not always cause a bug. That's why the linked Q&A says it's "almost certainly wrong". Sometimes the logic inside the loop is specifically designed to work correctly; other times it just so happens to avoid the bug, as yours does.

The correct code is:

while (file >> text)
{
    offset = text.find(search, 0);
    if (offset != string::npos) {
        cout << text << endl;
    }
}

More about EOF in loop condition

1 Answers