How to read/write std::string values from/to binary files

The two lines:

outf.write( reinterpret_cast<char *>( &temp ), sizeof( Item ) );

and

inf.read( reinterpret_cast<char *>( &temp ), sizeof( Item ) );

are wrong. You are writing the binary layout of the object, including std::string instances. This means you are writing the value of pointers to a file and reading them back.

You cannot simply read pointers from a file and assume they point to valid memory, especially if it was held by a temporary std::string instance, which should have freed the memory in it’s destructor when it went out of scope. I’m surprised you got this to run “correctly” with any compiler.

Your program should write the content and read it back using your operator<< and operator>> methods. It should look like the following:

void write_to_file( const string& fn, const Item& item )
{
    fstream outf( fn.c_str(), ios::binary | ios::out );
    outf << item << std::endl;
    outf.close();
}

void read_from_file( const string& fn, Item& item )
{
    fstream inf( fn.c_str(), ios::binary | ios::in );
    if( !inf )
    {
        cout << "What's wrong?";
    }
    inf >> item;
    inf.close();
}

BONUS: There are a few quirks with your code.

This statement is, thankfully, presently unused in your program (the method is not called).

return ( string& )"";

It is invalid because you will be returning a reference to a temporary string object. Remeber that a string literal "" is not a std::string object and you can’t get a reference to it of type std::string&. You should probably raise an exception, but you could get away with:

string& operator []( int x )
{
    static string unknown;
    if ( 0 == x )
        return itemID;
    if ( 1 == x )
        return itemName;
    if ( 2 == x )
        return itemState;
    return unkonwn;
}

This is a poor solution given that the string is returned by reference. It can be modified by the caller, so it might not always return the "" value you thought it would. However, it will remove undefined behavior for your program.

The .close() method invocations on std::fstream objects are not necessary, the destructor calls it automagically when the object goes out of scope. Inserting the call there is extra clutter.

Also, what’s with the redundant naming convention?

class Item
{
private:
    string itemID;
    string itemName;
    string itemState;
// ...
};

What’s wrong with calling them ID, name and state?