Computernerd
Computernerd

Reputation: 7782

Need help debugging simple update binary file function

void updatebfile(char filename[MAX])
{

fstream writereadb;

char cont='y';
char filenameb [MAX];
int i=1;
int record;

student s;

strcpy(filenameb,filename);
strcat(filenameb,".dat");

writereadb.open(filenameb,ios::in | ios::out | ios::binary  );


cout<<"------------------------------"
    <<endl;

cout<<"Begin updating of binary file "
    <<filenameb
    <<endl
    <<endl;

cout<<"Information for student file"
    <<endl
    <<endl;


while ( writereadb.read (reinterpret_cast <char *>(&s), sizeof (s) ) )
{

    cout<<i
        <<'\t'
        <<s.identity
        <<" "
        <<s.name
        <<endl;

    i++;


}

do
{


cout<<endl
    <<"Update record: ";
cin>>record;

cout<<endl
    <<"Student id: ";



writereadb.seekg ((record - 1) * sizeof(s), ios::beg);//problem is here
writereadb.read (reinterpret_cast <char *>(&s), sizeof (s));//always reading last value


cout<<s.identity
    <<endl;





cout<<"Update the name: ";
cin>>s.name;



writereadb.seekp((record-1)*sizeof(student),ios::beg);  
writereadb.write (reinterpret_cast <const char *>(&s), sizeof (s));

cout<<"Any more update (y/n) :";
cin>>cont;

}while (cont=='y');

writereadb.close();







}

I have this simple function where i am suppose to update the binary file. The problem is that i cant seem to set the get pointer, i am always reading the last value in the binary file when i cout s.identity

Upvotes: 0

Views: 338

Answers (1)

Ulrich Eckhardt
Ulrich Eckhardt

Reputation: 17444

You always try to read one entry and only use the result if that succeeded (which is perfectly right). If it didn't succeed, e.g. because it encountered EOF, the streamstate will be set to "fail". This causes the reading loop to terminate, but it also causes any subsequent operation on that filestream to fail, until you explicitly reset the streamstate. Therefore, you need to call writereadb.clear() after that loop.

Some more notes:

  • Passing a char filename[MAX] will not pass an array to a function! Instead, it is the same as a char* filename, i.e. modifying that parameter will make it visible in the calling function. Use std::string, use their c_str() memberfunction to get a pointer for the fstream.
  • You didn't read the last value, you failed to read it! Your code should have detected that. Also, it seemed to have read the last value because you are reusing the temporary struct. In general, it's a good idea to keep the size of the scope of variables as low as possible. The habit of declaring all variables at the beginning of a function belongs to old times (last century) where this was necessary for C code, but not in C++ code. In this case, I could even imagine splitting the whole program into two or three functions.
  • Dumping structs to files is not portable between different computers, not even different compilers on the same computer sometimes. For that reason, there are multiple serialization libraries out there that do this correctly.

Upvotes: 2

Related Questions