Never ending file reading loop (C++)

Question

There's some code I wrote that should read all new lines in a text file, but it gets stuck on one of the loops it runs.

The code is as follows:

#define MAX_MESSAGE_LENGTH 200;

fstream("some/random/file.txt", ios::in | ios::out);
streampos fileSizeReference = 0;

vector<string> messages;
vector<string> onDisplay;

char message[MAX_MESSAGE_LENGTH];
if((int)fileSizeReference == 0)
    fileReader.seekg(0);
            
else
    fileReader.seekg((int)fileSizeReference + 1);
            
cout << "Test" << endl;
            
// Add all new messages to the messages vector
do
{
    fileReader.getline(message, MAX_MESSAGE_LENGTH);
    string newMsg = message;
    messages.push_back(newMsg.substr(0, MAX_MESSAGE_LENGTH));
}
while (!fileReader.eof());
//

cout << "Test" << endl;
            
fileReader.seekg(0, ios::beg);
            
// Set the newest messages in the onDisplay vector
for(int i = 0; i < amountOfMessages; i++)
{
    onDisplay[i] = messages[messages.size() - (i + 1)];
}
//
            
cout << "Test" << endl;
        
// Display new messages
int current_Y = 0;
        
for(int i = 0; i < amountOfMessages; i++)
{
    current_Y = renderText(messages[i], current_Y);
}
//
        
// set the new file size as the fileSizeReference
fileReader.seekg(0, ios::end);
fileSizeReference = fileReader.tellg();

The text file looks like this:

Hello World!
Carpe Diem
Random Message

Whenever I run this code, I don't get past the first do-while loop. This is the first run, so the fileSizeReference is 0.

The newMsg variable within the loop is always an empty string, just like the message array.

Does any one of you know why my code gets stuck? Thank you in advance!

Who told you to write this `!fileReader.eof()`? Because its wrong. — john, Jun 26 '20 at 12:42
This code does not compile. Do you have a [mcve]? The chances are high that if I fix up the code so it actually compiles, I'll fix the bug(s) along the way. — Eljay, Jun 26 '20 at 12:47
Serious question, we see this error all the time, so I'm keen to know why new programmers choose to code this way. — john, Jun 26 '20 at 12:47
You use `message` but you don't seem to test if there is anything usable there. Doesn't your `reader.getline()` return any success/failure status? Why not check it? — CiaPan, Jun 26 '20 at 12:48
Why is that? I learned to use this in combination with the >> operator and that worked fine. How come it doesn't work with .getline? — V.Molotov, Jun 26 '20 at 12:52
It doesn't work with operator>> either. It will *sometimes* work with either, it depends what is in the file. But the one thing it doesn't do is test if you are at the end of the file. It has a different purpose. — john, Jun 26 '20 at 12:53
See here https://en.cppreference.com/w/cpp/io/basic_ios/eof (read the notes especially) — john, Jun 26 '20 at 12:56
@john From my own experience, when I was just starting with C++ and didn't understand it, it seemed like exact right idea. I check if file is ended, no, I read some more. It's far from obvious that eof bit is only set after failed read and documentation on everything related to streams is horribly convoluted (because streams are horribly convoluted). And there are all the websites that teach it and don't mention that it's a trap. Or you just ignore that mention, because you finally found a solution and can copy and paste that incomprehensible snippet and it works! — Yksisarvinen, Jun 26 '20 at 13:05
@CiaPan I will definately include that! I just want to have a working prototype first. As soon as that's done, I'll get to implementing your wonderful suggestion — V.Molotov, Jun 26 '20 at 15:07
I dare to suspect you're not going to have a working prototype unless you include it. The `eof` state is **not** set when you read the last byte from the stream, but rather when you try (and fail!) to read the first byte **past the end** of the stream. http://www.cplusplus.com/reference/ios/ios_base/iostate/ So, **if** you expect to have an `eof` state at the end of your loop, it **must** be a result of a `getline` failure inside the loop. — CiaPan, Jun 26 '20 at 16:01

john · Accepted Answer · 2020-06-26T12:51:39.517

You have two errors (at least).

do
{
    fileReader.getline(message, MAX_MESSAGE_LENGTH);
    string newMsg = message;
    messages.push_back(newMsg.substr(0, MAX_MESSAGE_LENGTH));
}
while (!fileReader.eof());

should be

while (fileReader.getline(message, MAX_MESSAGE_LENGTH))
{
    string newMsg = message;
    messages.push_back(newMsg.substr(0, MAX_MESSAGE_LENGTH));
}

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

Incidentally those two lines in the while loop can also be simplified, just this will work perfectly well.

    messages.push_back(message);

And secondly after finishing reading the stream will be in an error state, which you need to clear before seeking back to the beginning of the file (closing and reopening the file would also work).

while (fileReader.getline(message, MAX_MESSAGE_LENGTH))
{
    string newMsg = message;
    messages.push_back(newMsg.substr(0, MAX_MESSAGE_LENGTH));
}
fileReader.clear();
fileReader.seekg(0, ios::beg);

Thank you for the added elaboration on the !fileReader.eof() — V.Molotov, Jun 26 '20 at 13:20

Never ending file reading loop (C++)

1 Answers1