wrong output searching in binary files using seekg() & seekp()

Question

I wrote this very simple code but its search section does not work.

I just saved the names and ages of the people in a binary file and I want to find them , it gives me the wrong record.

it's search part:

fstream myfile(filename , ios::binary | ios::out | ios::in);

data d;


int searchage;


cout << "age : ";
cin >> searchage;


myfile.seekg(sizeof(data)* (searchage) );

myfile.read((char*)&d , sizeof(data));

myfile.close();

cout << "name : " << d.getname() << '\t' << d.getage() << endl;

you may need the class :

    class data{

private:
    int age;
    char name[15];


public:


    data (){
    // null

    }
    void setname(string tempname){
    int len = tempname.size();
    len = (len < 15 ? len : 14);
    tempname.copy(name , len);

    name[len] = '\0';

    }

    string getname(){
    return name;
    }



    void setage(int tempage){
    age = tempage;
    }


    int getage(){

    return age;
    }

};

and saving part :

fstream myfile(filename , ios::binary | ios::out | ios::app);


data d;

string name ;
int age;


cout << "name : ";
cin >> name;

cout << "age : ";
cin >> age;


d.setname(name);
d.setage(age);



myfile.seekp(sizeof(data) * age , ios::beg);

myfile.write((char*)&d , sizeof(data));

myfile.close();

also whole code is here

    #include <iostream>
#include <fstream>
#include <iomanip>


using namespace std;

#define filename "D:\\data.dat"


class data{

private:
    int age;
    char name[15];


public:


    data (string tempname = "null" , int tempage = 1){

    setname(tempname);
    setage(tempage);

    }
    void setname(string tempname){
    int len = tempname.size();
    len = (len < 15 ? len : 14);
    tempname.copy(name , len);

    name[len] = '\0';

    }

    string getname(){
    return name;
    }



    void setage(int tempage){
    age = tempage;
    }


    int getage(){

    return age;
    }

};



int main(){

 int a;

    cout << "1 - save \n2 - search \nchose : ";
   
    cin >> a;

    if(a == 1){

        fstream myfile("D:\\data.dat", ios::out | ios::in | ios::app);


        data d , d2;

        if(!myfile)
        {
            cout << "creat file \n";
            ofstream myfile2(filename , ios::out | ios::binary);

            myfile2.write((char*)&d2 , sizeof(data));

            myfile2.close();
        }

        fstream myfile3(filename , ios::binary | ios::out | ios::in | ios::app);


        string name ;
        int age;


        cout << "name : ";
        cin >> name;

        cout << "age : ";
        cin >> age;


        d.setname(name);
        d.setage(age);



        myfile3.seekp(sizeof(data) * age , ios::beg);

        myfile3.write((char*)&d , sizeof(data));

        myfile3.close();

    }else if(a == 2){
        fstream myfile(filename , ios::binary | ios::out | ios::in);

        data d;


        int searchage;


        cout << "age : ";
        cin >> searchage;


        myfile.seekg(sizeof(data)* searchage );

        myfile.read((char*)&d , sizeof(data));

        myfile.close();

        cout << "name : " << d.getname() << '\t' << d.getage() << endl;


    }







}

I don't know what's wrong.

Answer 1

First, there is no way the code above will compile without error or warning. Specifically due to your use of:

    void setname(string tempname){
      ...
      tempname.copy(name , len);

The .copy() member function is not a member of std::string, but instead a member of std::basic_string_view (C++17). The compiler does not know what .copy() is in that circumstance. Instead, you need to use std::basic_string_view<char> as the type for tempname, e.g.

  void setname (std::basic_string_view<char> tempname) {
    int len = tempname.size();
    len = (len < 15 ? len : 14);
    tempname.copy(name , len);
    name[len] = '\0';
  }

(note: len should be type size_t)

Your constructor can use the same approach to fill name, e.g.

  data (std::basic_string_view<char> tempname = "null" , int tempage = 1) : 
  age{tempage} {
    tempname.copy (name, tempname.size());
  }

While use of char[] instead of std::string should be avoided, it is clear that is used in data to ensure a fixed-size object with one 15-char C-string and one int)

Basic Problem with .seekp() and .seekg() - age Is Not An Offset

As mentioned in the comment above, when you save, unless D:\\data.dat has at least age number of objects already stored in the file, your save and search will attempt to move the file position indicator beyond the end of the file with. This will absolutely occur if you attempt to search before a save when your file doesn't exist. For example:

    myfile3.seekp (sizeof(data) * age , std::ios::beg);

In the case of if (!myfile) { ... } there is only a single data object written to the file. That is created here:

      myfile2.write((char*)&d2 , sizeof(data));

That writes d2 only to the file.

In the case of search, the same problem applies. When you read searchage you have no guarantee that there are at least that many data objects in the file. You need some way to ensure you have valid data to read. As mentioned in the comment, one way to ensure that seekp() and seekg() succeed is to check the stream-state (the return) from each call. See std::basic_ostream::seekp (and the same for seekg())

For example, after a single output is written to the file, attempting to search will attempt to seekg() beyond the end of the file with:

    myfile.seekg (sizeof(data)* searchage );

Checking if failbit is set will tell you if your seek succeeded. As it currently is, seekg() leaves the read-position indicator at the end of the file and you read nothing. (but since you don't check the stream state after the read -- you likewise don't know it is failing)

(simply commenting out the seekg() call will allow you to read your first record back in)

To fix the issue with seekg(), you need to keep track of the number of data objects available in your file (say n) and only offset at most (n - 1) * sizeof(data) bytes from the beginning. If overwriting existing data, then you need to limit seekp() by the same amount, otherwise just seek the end of the file to write a new object to file.

Without your sample input, there is no way to test your file. However, fixing the issues above your code does create an empty file and writes name and age to the file that after commenting out your seekg() call are read back into the program into d just fine.... For example, changing your search code to the following minimally validated checks for seekg() and read() will allow you to read the record number (data object number) from your file. record is used instead of searchage below:

  else if (a == 2) {
    std::fstream myfile (filename , std::ios::binary | std::ios::out | 
                                    std::ios::in);

    data d;
    int record;

    std::cout << "record no. : ";
    if (!(std::cin >> record)) {
      std::cerr << "error: invalid input - record.\n";
      return 1;
    }

    if (!myfile.seekg (sizeof(data)* (record - 1))) {
      std::cerr << "error: seekg() failed.\n";
      return 1;
    }

    if (!myfile.read ((char*)&d , sizeof(data))) {
      std::cerr << "error: read failed.\n";
      return 1;
    }

    myfile.close();

    std::cout << "name : " << d.getname() << '\t' << d.getage() << '\n';
  }

Example Use/Output

With two records written to a data file ("Henry", 10 and "Mike", 20), the data can be retrieved without any problems using the correct record offset, e.g.

$ hexdump -Cv dat/nameage.dat
00000000  0a 00 00 00 48 65 6e 72  79 00 f8 2e fd 7e 00 00  |....Henry....~..|
00000010  01 c9 d9 2e 14 00 00 00  4d 69 6b 65 00 5e 7b 17  |........Mike.^{.|
00000020  2c 7f 00 00 01 b9 5c 17                           |,.....\.|
00000028

The first can be read with:

$ ./bin/name_age
1 - save
2 - search
chose : 2
record no. : 1
name : Henry    10

and the second with:

$ ./bin/name_age
1 - save
2 - search
chose : 2
record no. : 2
name : Mike     20

Attempting to read more data than exists, generates an error, e.g.

$ ./bin/name_age
1 - save
2 - search
chose : 2
record no. : 3
error: read failed.

Working Example - Retrieve No. of data Records from File

Cleaning up the code a bit so you only need open a single file and getting the size in bytes to compute the number of data records available in the file so you can prevent attempting to seek beyond the end of your file, you could do something similar to the following. Note you must set your compiler language standard to C++17. Also note, with the number of data records known -- there is no reason to write an empty data struct to the file. You simply add a check to your search code to check if the waned records is available, and handle the error if it is not:

/* requires compiling with the C++17 language standard */
#include <iostream>
#include <fstream>
#include <iomanip>
#include <string_view>

#define filename "D:\\data.dat"
#define MAXNAME  15

class data {

 private:
  int age;
  char name[MAXNAME];

 public:

  data (std::basic_string_view<char> tempname = "null" , int tempage = 1) : 
  age{tempage} {
    tempname.copy (name, tempname.size());
  }
  
  void setname (std::basic_string_view<char> tempname) {
    int len = tempname.size();
    len = (len < 15 ? len : 14);
    tempname.copy(name , len);
    name[len] = '\0';
  }

  std::string getname() { return name; }
  void setage (int tempage) { age = tempage; }
  int getage() { return age; }
};

int main() {

  int a;
  size_t nrecords = 0;        /* number of records in file */
  data d;

  /* open file in r/w, bin, app mode (a+b), will create if doesn't exist */
  std::fstream myfile (filename, std::ios::out | std::ios::in | 
                                 std::ios::app | std::ios::binary);

  if (!myfile.is_open()) {    /* validate file is open */
    std::cerr << "error: file open/create failed.\n";
    return 1;
  }
  
  myfile.seekp (0, std::ios::end);            /* seek to end */
  nrecords = myfile.tellp() / sizeof(data);   /* get number of records */
  
  std::cout << "File number of records : " << nrecords << "\n\n";
  
  if (!nrecords)  /* if no records, new file */
    std::cout << "creat file \n";   /* optional - your output */
  
  std::cout << "1 - save \n2 - search \nchose : ";  /* menu */
  
  if (!(std::cin >> a) || a < 1 || 2 < a) {   /* validate EVERY input */
    std::cerr << "error: invalid input.\n";
    return 1;
  }

  if (a == 1) {     /* save */
    
    std::string name {};
    int age;

    std::cout << "name : ";
    if (!(std::cin >> name)) {    /* validate EVERY input */
      std::cerr << "error: name not read.\n";
      return 1;
    }

    std::cout << "age : ";
    if (!(std::cin >> age)) {     /* validate EVERY input */
      std::cerr << "error: invalid input - age.\n";
      return 1;
    }

    d.setname(name);          /* set class members */
    d.setage(age);
    
    /* validate EVRY write */
    if (!myfile.write ((char*)&d , sizeof(data))) {
      std::cerr << "error: write failed.\n";
      return 1;
    }

  }
  else if (a == 2) {    /* search */
    
    if (nrecords == 0) {      /* validate records available to search */
      std::cout << "file-empty, nothing to search.\n";
      return 1;
    }
    
    size_t record;            /* record number to read */

    std::cout << "record no. : ";
    if (!(std::cin >> record)) {  /* validate EVERY input */
      std::cerr << "error: invalid input - searchage.\n";
      return 1;
    }
    
    if (record > nrecords) {  /* check requested record in range */
      std::cerr << "error: requested record out-of-range.\n";
      return 1;
    }
    
    /* validate seek from beginning */
    if (!myfile.seekg (sizeof(data) * (record - 1))) {
      std::cerr << "error: seekg() failed.\n";
      return 1;
    }
    
    /* validate EVERY read */
    if (!myfile.read ((char*)&d , sizeof(data))) {
      std::cerr << "error: read failed.\n";
      return 1;
    }

    std::cout << "name : " << d.getname() << '\t' << d.getage() << '\n';
  }
}

General Observations

You need to validate all user-inputs by checking the stream-state after the read. That can be as simple as:

  if (!(std::cin >> a)) {   /* validate EVERY input */
    std::cerr << "error: invalid input.\n";
    return 1;
  }

or you can include the value check for a as well, e.g.

  if (!(std::cin >> a) || a < 1 || 2 < a) {   /* validate EVERY input */
    std::cerr << "error: invalid input.\n";
    return 1;
  }

Including the entire standard namespace in your code is also discouraged. See: Why is “using namespace std;” considered bad practice?. Though for an exercise it doesn't hurt, just be aware it should be avoided moving forward.

I'm sure there are additional error that are unable to be tested without input, but those above are the primary issues that will prevent your code from working.