-11
#include<fstream>
#include<iostream>
using namespace std;
class employee
{
        char *name;
        int age;
        string designation; // string data type is used
        float salary;
    public:
        void getdata();
        void display();
};
void employee::getdata() // for taking the input 
{
    cout<<"\nENTER THE NAME OF THE EMPLOYEE: ";
    gets(name); /*name is a char pointer */
    cout<<"\nENTER THE AGE OF THE EMPLOYEE: ";
    cin>>age;
    cout<<"\nENTER THE DESIGNATION OF THE EMPLOYEE: ";
    getline(cin,designation); /*designation is string data type*/
    cout<<"\nENTER THE SALARY OF THE EMPLOYEE: ";
    cin>>salary;
}
void employee::display()//for displaying the inputed data
{
    cout<<"\nTHE NAME OF THE EMPLOYEE: ";
    puts(name);
    cout<<"\nENTER THE AGE OF THE EMPLOYEE: ";
    cout<<age;
    cout<<"\nENTER THE DESIGNATION OF THE EMPLOYEE: ";
    cout<<designation;
    cout<<"ENTER THE SALARY OF THE EMPLOYEE: ";
    cout<<salary;
}
int main()
{
    ofstream fout;
    char ch;
    fout.open("employee.txt",ios::out|ios::binary);/*use of open function in file handing*/
    employee e;
     for(int i=0;i<3;i++)
     {
        e.getdata();
        fout.write((char*)&e,sizeof(e));//write() is used
     }
     fout.close();
     ifstream fin;
     fin.open("employee.txt",ios::in|ios::binary);
     int j;
     cout<<"\n Enter the employee no. to be read ";
     cin>>j;
     fin.seekg((j-1)*sizeof(e));
     fin.read((char*)&e,sizeof(e));
     cout<<"\n Details of employee no. of object is = ";
     e.display();
     return 0;
}

I am not able to identify the error in my code...I had cross checked the code...there is no syntax error and no compiler error but the output is not correct...it is not taking the proper input from the user.

  • 9
    This is not debugmycodeforme.com – Lightness Races in Orbit Feb 29 '16 at 16:45
  • 5
    `char *name;` In `c++` you should use a string instead. If that is not permitted you need to allocate memory for name. – drescherjm Feb 29 '16 at 16:45
  • 1
    Not the issue you have right now with getting the name but once you figure that out then you will have to deal with [this](http://stackoverflow.com/questions/15176953/c-read-and-write-classes) – NathanOliver Feb 29 '16 at 16:48
  • 1
    Please explain what you mean by "not taking the proper input." – Charlie Feb 29 '16 at 16:52
  • 2
    Although that's not the immediate issue either, `gets()` is deprecated. Chances are your compiler will refuse to compile `gets()` in about two years, unless you explicitly tell it to. The simplest safe method for input is probably to just read a C++ `std::string` with `>>`. (You'll find out why it is deprecated once you realize that you need to provide a buffer for the C strings which is large enough to hold the longest name which anybody ever will want to enter. What's the world-wide upper limit for a name length?) – Peter - Reinstate Monica Feb 29 '16 at 16:54
  • At first it is not showing the compiler error, it is executing...but it skips to take the input of name and designation. – V Surya Pushpa Feb 29 '16 at 16:57
  • Cf. https://answers.yahoo.com/question/index?qid=20090226172103AApU8Op for an idea about name lengths ;-). – Peter - Reinstate Monica Feb 29 '16 at 16:59
  • The more serious aspect of `gets()` is that every C program exposed to the world must be prepared for malicious input. That's plain impossible when using `gets()` -- whatever your buffer length is, a malicious user can enter a longer name. The best case is that it crashes your program. The worst case is that the malicious user overwrites the return address on the stack and injects code which gives it a shell under your name. I know that's currently not a problem for you, and that you have a few more pressing problems; but start best practice right away. :-) – Peter - Reinstate Monica Feb 29 '16 at 17:03
  • it means that if I specify the length of the char and name, it will run perfectly? – V Surya Pushpa Feb 29 '16 at 17:07
  • The `gets` has a buffer overrun possibility. Use `fgets` instead and supply the capacity (limit) of the character array. – Thomas Matthews Feb 29 '16 at 17:08
  • I would recommend making sure age and salary entered are numbers. And maybe adding info about what output you are expecting considering a given input.... – ahesa Feb 29 '16 at 17:08
  • Don't cross the streams. Use `cout << name` and not `puts(name)`. Choose one output stream type and *stick with it*. – Thomas Matthews Feb 29 '16 at 17:10
  • it is directly taking the input of age and salary which are numbers thereby skipping to take the input of name and designation which are char or string. – V Surya Pushpa Feb 29 '16 at 17:10
  • Use `std::string` for the names and `std::getline(cin, name)` to read them. – Thomas Matthews Feb 29 '16 at 17:11
  • as I had used 'using namespace std' then why to use std::string and std::getline again. – V Surya Pushpa Feb 29 '16 at 17:14
  • Don't block write (`fout.write`) for structures that contain `std::string` or other Non-POD types. Most likely, you are writing the header information of the `designation` member and not the text content. The `std::string` is allowed to have pointers to the text (the text may be on the heap). Likewise, you can't use `fin.read` method either. – Thomas Matthews Feb 29 '16 at 17:14
  • Also, don't binary (block) write pointers. For example, the `fout.write` statement will write the value in the pointer `name`, but not what the pointer points to. There is no guarantee that the OS will load your program in the *exact* memory location when you next run it; thus the value of the pointer you saved will not be valid. – Thomas Matthews Feb 29 '16 at 17:17
  • Note that the `sizeof(employee)` does not include the length of the `name` text nor the contents of the `designation` string. Try it. You'll find that the length is constant. BTW, your employee structure has variable length, and you cannot use the `seekg` method, like an array, with variable length records (unless you have a container of each record's starting file position). – Thomas Matthews Feb 29 '16 at 17:21
  • can u help me to change the code or tell me that how to give the limit to the string and pointer pointing to char – V Surya Pushpa Feb 29 '16 at 17:23
  • @VSuryaPushpa "*can u help me to change the code or tell me that how to give the limit to the string and pointer pointing to char*" -- Honestly, there is so much wrong with your code, it is difficult to know where to start correcting it. Everything from erroneously using `sizeof`, to writing non-POD types to a file as if they are `C` types, etc. First, if you're going to write to a binary file, you write the **data** to the file, not pointer values, and not the set of bytes that make up an object. – PaulMcKenzie Feb 29 '16 at 18:03
  • @VSuryaPushpa -- If you took the file you wrote and opened it up in an editor, could *you* understand it? I bet not. So why would you think that your read code will work if your own eyes tells you that the file is bad, or at least, looks weird? I say this to all the newbies (you're not the first) that insist on writing files in binary mode, and almost always, read back garbage. – PaulMcKenzie Feb 29 '16 at 18:07

2 Answers2

1

In addition to what the comments say about gets being dangerous, you immediately run into that buffer overflow by not allocating any space for name. Just having a pointer is not enough.

In addition to that, mixing cin >> and getline(cin,...) is known to skip input, because the two functions handle end-of-line differently.

Then we have the problem of doing binary I/O for the employee type. In general, you cannot do that for class types that are non-trivial. Specifically, the std::string designation member internally holds a pointer to some data. That pointer will not survive the transfer to disk and back.

Community
  • 1
  • 1
Bo Persson
  • 86,087
  • 31
  • 138
  • 198
1
  • As Bo Persson said, your program does not provide any memory for name and designation. You declare pointers like name in your class alright, but they do not point to anything. You could just declare e.g. name as char name[100]; and hope that 100 is enough (and, in production code, add checks to ensure it isn't exceeded).

    But in C++ there is the string class which frees you of many worries. In particular, it makes input of arbitrarily long strings easy. If the string can't have whitespace in it, a simple string s; cin >> s; reads a "word" from the input into the string. If it can have whitespace, you need some way to tell where it starts and ends. Databases, Excel etc. often use quotes to surround a string. If it cannot contain newlines, ensuring that it is on a line of its own is sufficient and obviates the need for parsing. That's what we'll do here.

  • The second issue you faced is what's technically called "serialization". You want to store ("persist") your class on disk and later (much later, perhaps) re-read it into a new instance. Nathan Oliver pointed you to a resource which discusses serialization. For a simple class like employee with a limited number of simple data members we can simply roll our own ad-hoc serialization: We write everything to disk with operator<<(), and read everything back with operator>>().

  • The main thing to consider is that strings may have whitespace, so that we'll put them on a line of their own.

  • An addition is that in order to be more robust when reading from a file we start each employee with a start marker (header in the code below). This way reading an employee will work from any position in the file. Also, if later employees should have more fields we can still read our basic employee data and just skip the additional fields before we read the next employee in a sequence of employees on disk.

  • streams are closed automatically when they are destroyed at the end of their scope; we use block scope for that (check the additional {} in the code).

  • A plain float´ is not precise enough for higher salaries (it only has about 7 decimal digits, so that for salaries > 167772.16 (if I can believe Wikipedia) in whatever currency the pennies start to fall off the cliff). I use long double` and make sure to not lose precision when converting it to text.

  • You didn't compare reals but I did in order to check whether I read the employee back correctly. Care must be taken there. I wiggle out of the gory details by comparing half pennies which should be appropriate for money.

Here is the whole program. (Compared with my previous version I have simplified the (de)serialization, in particular I have eliminated the useless tags.)

It would be wise to perform error checking after each read/write in order to make sure it succeeded; remember, a stream coverts to bool, so a simple if(!os) { cerr << "oops" << endl; /* exit? */} is enough; but I didn't want to distract from the actual program too much.

#include <iostream>
#include <string>
#include <sstream>
#include <fstream>
#include <cfloat>   // for LDBL_DIG in ostream precision.
#include <cstdlib>  // for exit()
using namespace std;


/** A simple class holding employee information */
class employee
{   
    public:
 
        /** This header starts each 
            "serialization" of an employee on a line
            of its own. */
        static constexpr const char *header = "--- Employee ---";
        
        // use C++ std::string
        string name;

        int age;

        // use C++ std::string
        string designation;

        // Be as precise as possible,
        // for later uses like salary increases
        // by 4.5% or so :-)
        // The salary is in units like USD or EUR.
        // The fraction part is pennies, and fractions of them.
        long double salary;

    public:
        void readdata(istream &is);
        void writedata(ostream &os);
        void display();

        bool operator==(employee &rhs)
        {   return 
                name == rhs.name 
            &&  age == rhs.age 
            &&  designation == rhs.designation 
            
            // Do not compare floats directly.
            // We compare pannies, leaving slack for rounding.
            // If two salaries round to the same penny value,  
            // i.e. 0.01, they are equal for us.
            // (This may not be correct, for an accounting app,
            // but will do here.)
            &&  salary - rhs.salary < 0.005 
            &&  rhs.salary - salary < 0.005;
        }
};

/** Write a header and then 
    each data member in declaration order, converted to text,
    to the given stream. The header is used to find the start
    of the next employee; that way we can have comments or other
    information in the file between employees.
    The conversion is left to operator<<.
    Each member is written to a line of its own, so that we
    can store whitespace in them if applicable.
    The result is intended to be readable by @readdata().
*/
void employee::writedata(ostream &os)
{           
    os.precision(LDBL_DIG); // do not round the long double when printing
    
    // make sure to start on a new line....
    os                  << endl
        // ... write the header on a single line ...
        << header       << endl
        // ... and then the data members.
        << name         << endl
        << age          << endl
        << designation  << endl
        << salary       << endl;
}

/** 
    Read an amployee back which was written with @writedata().
    We first skip lines until we hit a header line,
    because that's how an employee record starts.
    Then we read normal data mambers with operator>>. 
    (Strictly spoken, they do not have to be on lines
    of thier own.)
    Strings are always on a line of their own,
    so we remove a newline first.
*/
void employee::readdata(istream &is)
{
    string buf;

    
    while(getline(is, buf)) // stream converts to bool; true is "ok"
    {
        if( buf == header) break; // wait for start of employee
    }
    if( buf != header ) 
    { 
        cerr << "Error: Didn't find employee" << endl;
        return;
    }
    
    getline(is, name);  // eats newline, too
    is >> age;          // does not eat newline:
    
    // therefore skip all up to and including the next newline
    is.ignore(1000, '\n');
    
    // line on its own, skips newline
    getline(is, designation);
    
    is >> salary;
}

int main()
{
    const char *const fname = "emp.txt";
    
    employee empa;
    empa.name = "Peter A. Schneider";
    empa.age = 42;
    empa.designation = "Bicycle Repair Man";
    empa.salary = 12345.67;

    employee empb;
    empb.name = "Peter B. Schneider";
    empb.age = 43;
    empb.designation = "Bicycle Repair Woman";
    empb.salary = 123456.78;

    {
        ofstream os(fname);
        if(!os) 
        { 
            cerr << "Couldn't open " 
            << fname << " for writing, aborting" << endl;
            exit(1);
        }

        empa.writedata(os);
        cout << "Employee dump:" << endl;
        empa.writedata(cout);

        // insert a few funny non-employee lines
        os << endl << "djasdlköjsdj" << endl << endl;
        
        empb.writedata(os);
        cout << "Employee dump:" << endl;
        empb.writedata(cout);
    }

    // show the file contents
    {
        ifstream is(fname);
        if(!is) 
        { 
            cerr << "Couldn't open " 
                << fname << " for reading, aborting" << endl;
            exit(1);
        }

        string line;
        cout << "-------------- File: -------------" << endl;
        while(getline(is, line)) cout << line << endl;
        cout << "---------------End file ----------" << endl;
    }
    /////////////////////////////////////////////////////////

    {
        ifstream is(fname); // read from the file "emp.txt" just written
        if(!is) 
        { 
            cerr << "Couldn't open " 
                << fname << " for reading, aborting" << endl;
            exit(1);
        }

        
        {
            employee emp2;  // new employee, sure to be empty
            cout << endl << "Re-reading an employee..." << endl;
            emp2.readdata(is);

            cout << endl << "Re-read employee dump:" << endl;
            emp2.writedata(cout);

            cout << "Original and written/read employee are " 
                << (empa == emp2 ? "" : "NOT ") << "equal" << endl;
        }
    
        {   
            employee emp2;  // new employee, sure to be empty
            
            // now read next employee from same stream.
            // readdata() should skip garbage until the header is found.
            cout << endl << "Re-reading an employee..." << endl;
            emp2.readdata(is);

            cout << endl << "Re-read employee dump:" << endl;
            emp2.writedata(cout);

            cout << "Original and written/read employee are " 
                << (empb == emp2 ? "" : "NOT ") << "equal" << endl;
        }
    }
}

Sample session:

Employee dump:

--- Employee ---
Peter A. Schneider
42
Bicycle Repair Man
12345.6700000000001
Employee dump:

--- Employee ---
Peter B. Schneider
43
Bicycle Repair Woman
123456.779999999999
-------------- File: -------------

--- Employee ---
Peter A. Schneider
42
Bicycle Repair Man
12345.6700000000001

djasdlköjsdj


--- Employee ---
Peter B. Schneider
43
Bicycle Repair Woman
123456.779999999999
---------------End file ----------

Re-reading an employee...

Re-read employee dump:

--- Employee ---
Peter A. Schneider
42
Bicycle Repair Man
12345.6700000000001
Original and written/read employee are equal

Re-reading an employee...

Re-read employee dump:

--- Employee ---
Peter B. Schneider
43
Bicycle Repair Woman
123456.779999999999
Original and written/read employee are equal
Community
  • 1
  • 1
Peter - Reinstate Monica
  • 12,309
  • 2
  • 29
  • 52