Simple File processing in C++

Question

Trying to read a record of students with their name and numbers. Writing to the file seems fine. However, reading from it prints a never-ending output. The statement - while(!file.eof()) - is causing the problem. But it's how I read the remaining person_details. Your help would be greatly appreciated.

#include <iostream>
#include <fstream>
#include <string>
using std::cin;
using std::cout;
using std::endl;
using std::fstream;
using std::string;
using std::ios;


class telephone
{
protected:
    string name;
    int number; 
public:
    void getDetails();
    void printDetails() const;
};


void telephone:: getDetails()
{
    cout<<"Enter name : "; getline(cin,name);
    cout<<"Enter number : ";cin>>number;
}

void telephone:: printDetails() const
{
    cout<<"Name : "<<name<<endl;
    cout<<"Number : "<<number<<endl;
}


int main(int argc, char const *argv[])
{
    telephone person;
    fstream file("telefile.txt",ios::in | ios::out | ios::binary | ios::app);
    if (!file)
    {
        cout<<"Invalid file name."<<endl;
        return 1;
    }
    //writing
    char choice;
    do{
        cout<<"----------"<<endl;
        cout<<"Person : "<<endl;
        cout<<"----------"<<endl;
        person.getDetails();
        file.write(reinterpret_cast<char*>(&person),sizeof(person));
        cout<<"Enter one more?";    
        cin>>choice;cin.ignore();
    }while(choice == 'y');

    //reading
    file.seekg(0);                    
    file.read(reinterpret_cast<char*>(&person),sizeof(person));
    while(!file.eof())
    {
        cout<<"----------"<<endl;
        cout<<"Person : "<<endl;
        cout<<"----------"<<endl;
        person.printDetails();
        file.read(reinterpret_cast<char*>(&person),sizeof(person));
    }
    return 0;
}

You said `while (!eof())` was causing the problem - *what's* the problem? — David G, Aug 26 '14 at 22:41
You shouldn't reinterpret a class with a `std::string` member as an array of characters. Do so only for POD types. — David G, Aug 26 '14 at 22:43
`while (!file.eof())` doesn't *cause* a problem. It *is* a problem. — Jerry Coffin, Aug 26 '14 at 22:43
[`while (!f.eof())` is ALWAYS wrong](http://stackoverflow.com/questions/5431941/while-feof-file-is-always-wrong) — The Paramagnetic Croissant, Aug 26 '14 at 22:45
@TheParamagneticCroissant: That's actually a really bad link, because all the information there is about and the `feof` function, which is different from `fstream::eof()` Some of the same reasoning applies, but none of the code there is even the slightest bit useful here. — Ben Voigt, Aug 26 '14 at 22:48
http://stackoverflow.com/q/21647/103167 and http://stackoverflow.com/q/5984789/103167 And note that the code in this question IS testing `eof()` *after* attempting a read... still not as good as testing for a good stream via `while (file)`, but not making the naive mistake. — Ben Voigt, Aug 26 '14 at 22:51

score 1 · Accepted Answer · edited Jun 20 '20 at 09:12

Your basic problem is that you're making a binary image of a std::string object, but a string object doesn't contain the character data, only a pointer to it (this is what allows a string to vary in size). So when you read that pointer back in, you'll start accessing whatever is in that memory location now, not what that memory held when you wrote the file. oops. Even worse things will happen when the string destructor runs and tries to free that pointer.

What you should do instead is write person.name.size() to the file, followed by that many bytes starting at &person.name[0]. Then when you read in the size, you can person.name.resize(size_from_file) and then read that many bytes into &person.name[0].

The Standard actually has a formal name for data types you are allowed to take binary images of: trivially copyable. The requirements are set out in section 9 and your telephone type doesn't meet them:

A trivially copyable class is a class that:

has no non-trivial copy constructors,

has no non-trivial move constructors,

has no non-trivial copy assignment operators,

has no non-trivial move assignment operators, and

has a trivial destructor.

The compiler-generated special member functions for telephone are non-trivial because of the std::string member, so instead of meeting all five requirements, you don't meet any of them.

Everyone has mentioned while (!file.eof()). You're not actually doing it wrong, but there is room for improvement.

First, you can catch more errors using while (file.good()) instead. Right now if you have any failure other than EOF, your loop never terminates, which matches your symptom.

Next, streams have a conversion to bool which is equivalent to calling good(), so you can write while (file). Finally, read like most other stream I/O functions returns the original stream, so you can write

while (file.read(buffer, size))

and avoid duplicating the read call both above and inside the loop.

But you didn't fall into the very common trap of checking eof() before doing the read that actually ran into the end. Bravo for that.

Simple File processing in C++

1 Answers1