-1

I have written a function to read from .txt file and build a linked list, but the function missed the first line of the file. Anyone have any idea why that happens?

output:

//empty line  
gmail  
San Francisco  
123456

.txt file

person1
gmail
San Francisco
123456

readFile function

void list::readFile(std::string fileName){
    fstream fs;
    string dummy = "";
    string firstline;
    record * previous = new record;
    record * temp = new record;
    int recordCount = 0;
    fs.open(fileName, ios::in | ios::binary);
        do{
            std::getline(fs, temp->name);
            std::getline(fs, temp->email);
            std::getline(fs, temp->address);
            fs >> temp->phoneNo;
            std::getline(fs, dummy);
            recordCount++;

            if(head == NULL){
                head = temp;
                previous = temp;
                tail = temp;
            } else {
                previous->next = temp;
                previous = temp;
                tail = temp;
            }

        } while(!fs.eof());
    // } else {
    //     cout << "there's no record in the file yet." << endl;
    // }

    cout << head->name << endl;
    cout << head->address <<endl;
    cout << head->email << endl;
    cout << head->phoneNo << endl;
}
Alan Birtles
  • 32,622
  • 4
  • 31
  • 60
Nigel
  • 161
  • 12
  • Unrelated, you're leaking the allocation made here: `record * previous = new record;`. This isn't Java/C#. You don't have to 'new' the world. Regarding your code, I'm curious why you're opening the file in binary-mode, yet reading it as formatted text. Also, `while(!fs.eof())` is a bad move, regardless of whether it is in the control expression of a while-loop, or a do-while-loop. Every single one of your reads is *unchecked*. – WhozCraig Mar 09 '22 at 05:35
  • 2
    [Why is iostream::eof inside a loop condition (i.e. `while (!stream.eof())`) considered wrong?](https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-i-e-while-stream-eof-cons) – molbdnilo Mar 09 '22 at 05:38
  • @molbdnilo I think the answer lies here: https://stackoverflow.com/a/4533102/2347040 – sbecker Mar 09 '22 at 05:44
  • 1
    Also, you need one `new record` for every node you add to the list. Since `temp` never changes and you start with `previous = temp`, `previous->next = temp;`is the same as `temp->next = temp;`. – molbdnilo Mar 09 '22 at 05:44
  • works for me, but I am very suspicious of the ios::binary on the file open. Its all to do with line endings and I am on windows. – pm100 Mar 09 '22 at 05:45
  • sorry I forgot to mention, there's a cin call before this function called, the cin takes an int value then a switch parse the value to the readFile function, will that cin affects what is read by the getline in the function? – Nigel Mar 09 '22 at 05:58
  • No. your IO reads in the function is entirely against the opened file stream, `fs`. There is no extractions against `std::cin` – WhozCraig Mar 09 '22 at 06:01
  • This doesn't address the question, but get in the habit of initializing objects with meaningful values rather than default-initializing them and immediately overwriting the default values. In this case, that means changing `fstream fs; ... fs.open(fileName, ios::in | ios::binary);` to `fstream fs(fileName, ios::in | ios::binary);`. Or, better, `ifstream fs(fileName, ios::binary);`. – Pete Becker Mar 09 '22 at 14:58

2 Answers2

0

You create a single node so every iteration through the loop you overwrite the previously read values.

In the second iteration of the while loop your first call to getline presumably fails and sets the value of name to an empty string. As the stream is now in a failed state the rest of your reads are ignored and finally the loop breaks at the end of the loop.

This might be closer to what you wanted:

    int recordCount = 0;
    fs.open(fileName, ios::in | ios::binary);
        while(fs) {
            record * temp = new record;
            std::getline(fs, temp->name);
            std::getline(fs, temp->email);
            std::getline(fs, temp->address);
            fs >> temp->phoneNo;
            if (!fs) {
                // Reading failed, delete the temporary and exit the loop
                delete temp;
                break;
            }
            std::getline(fs, dummy);
            recordCount++;

            if(head == NULL){
                head = temp;
                previous = temp;
                tail = temp;
            } else {
                previous->next = temp;
                previous = temp;
                tail = temp;
            }

        }

Your code could be made memory leak safer by using smart pointers to ensure temp is never leaked even if exceptions are thrown.

Alan Birtles
  • 32,622
  • 4
  • 31
  • 60
  • for some reason while(fs) didn't work for my code, I end up using while(fs.peek() != EOF). – Nigel Mar 09 '22 at 12:26
0

I will explain to you why this problems happens and how it can be avoided.

It is a typical problem, mentioned thousands of times in comments, also above by "molbdnilo".

You must not use while (fs.eof()).

I will explain to you why. By the way, it only happens, because you have a new line at the end of the last line.

Let us see what happens:

  1. The variable "temp" is allocated one time
  2. The file is opened
  3. The do-loop starts
  4. You read the name, email, address via the unformatted input function std::getline. This function will eat up the trailing white space '\n' (but not cope it to the read variable
  5. You use the formatted input function >> for reading the phone number (This is not a good idea. Anyway). The formatted input function will stop at a white space, here the '\n' at the end of the last line. It will not consume the '\n'. This is still in the stream and wating for input.
  6. You use std::getline with a dunmmy, to read the still existing '\n' from the stream. The stream is still OK. Everything went well so far. The file is also not at EOF. We could read everything. So, fine.
  7. End of do loop. You say: while(!fs.eof()). But as said above: There is not yet an EOF. Everything was and still is OK. The loop will continue.
  8. Back at the start of loop again
  9. std::getline(fs, temp->name); will be called. And now and not before, we get an EOF. because we tried to read and now there is nothing more to read. "temp->name" will be set to an emptry string. And now, input stream is in condition EOF. 10 All further reads will do nothing, because the stream is in EOF. The variables will not be modified from now on.

That is the reason why, name is empty and the other data is still there from the last loop run.

For a quickfix you could write

void list::readFile(std::string fileName) {
    fstream fs;
    string dummy = "";
    string firstline;
    record* previous = new record;
    record* temp = new record;
    int recordCount = 0;
    fs.open(fileName, ios::in | ios::binary);
    if (fs) 
    while (std::getline(fs, dummy)) {
        temp->name = dummy;
        std::getline(fs, temp->email);
        std::getline(fs, temp->address);
        fs >> temp->phoneNo;
        std::getline(fs, dummy);
        recordCount++;

        if (head == NULL) {
            head = temp;
            previous = temp;
            tail = temp;
        }
        else {
            previous->next = temp;
            previous = temp;
            tail = temp;
        }

    } 
    // } else {
    //     cout << "there's no record in the file yet." << endl;
    // }

    cout << head->name << endl;
    cout << head->address << endl;
    cout << head->email << endl;
    cout << head->phoneNo << endl;
}

But this is also not good. I will show a better solution later.

A M
  • 14,694
  • 5
  • 19
  • 44