Reading/writing in binary files

[hornet07]

I'm a little confused about writing/reading in binary files, especially when it involves strings.

// Wiz.h

#ifndef WIZ_H
#define WIZ_H

#include <fstream>
#include <string>

class Wizard
{
public:
	Wizard();
	Wizard(std::string name, int hp, int mp, int armor);

	// [...] other methods snipped

	void print();

	void save(std::ofstream& outFile);
	void load(std::ifstream& inFile);

private:
	std::string mName;
	int mHitPoints;
	int mMagicPoints;
	int mArmor;
};

#endif


// Wiz.cpp

#include "Wiz.h"
#include <iostream>
using namespace std;

Wizard::Wizard()
{
	mName        = "Default";
	mHitPoints   = 0;
	mMagicPoints = 0;
	mArmor       = 0;
}

Wizard::Wizard(std::string name, int hp, int mp, int armor)
{
	mName        = name;
	mHitPoints   = hp;
	mMagicPoints = mp;
	mArmor       = armor;
}

void Wizard::print()
{
	cout << "Name = "  << mName        << endl;
	cout << "HP = "    << mHitPoints   << endl;
	cout << "MP = "    << mMagicPoints << endl;
	cout << "Armor = " << mArmor       << endl;
	cout << endl;
}

void Wizard::save(ofstream& outFile)
{
	outFile.write(mName.c_str(), mName.size());
	outFile.write((char*)&mHitPoints, sizeof(int));
	outFile.write((char*)&mMagicPoints, sizeof(int));
	outFile.write((char*)&mArmor, sizeof(int));
}

void Wizard::load(ifstream& inFile)
{
	string garbage;
	inFile.read((char*)&mName, mName.size()); // read name
	inFile.read((char*)&mHitPoints, sizeof(int)); // read hit points
	inFile.read((char*)&mMagicPoints, sizeof(int)); // read magic points
	inFile.read((char*)&mArmor, sizeof(int)); // read armor
}


// main.cpp

#include "Wiz.h"
#include <iostream>
using namespace std;

int main()
{
	// Create wizards with specific data.
	Wizard wiz0("Gandalf", 25, 100, 10);
	Wizard wiz1("Loki", 50, 150, 12);
	Wizard wiz2("Magius", 10, 75, 6);

	// Create a stream which will transfer the data from our program
	// to the specified file "wizdata.txt".
	ofstream outFile("wizdata.txt", ios_base::binary);

	// If the file opened correctly then call save methods.
	if (outFile)
	{
		// Dump data into the stream
		wiz0.save(outFile);
		wiz1.save(outFile);
		wiz2.save(outFile);

		// Done with stream -- close it.
		outFile.close();
	}



	// Create some blank wizards, which we will load data into
	Wizard wiz4;
	Wizard wiz5;
	Wizard wiz6;

	// Output the wizards before they are loaded.
	cout << "BEFORE LOADING..." << endl;
	wiz4.print();
	wiz5.print();
	wiz6.print();

	// Create a stream which will transfer the data from the 
	// file "wizdata.txt" to our program
	ifstream inFile("wizdata.txt", ios_base::binary);

	// If the file opened correctly
	if (inFile)
	{
		wiz4.load(inFile);
		wiz5.load(inFile);
		wiz6.load(inFile);
	}

	// Output the wizards to show the data was loaded correctly
	cout << "AFTER LOADING..." << endl;
	wiz4.print();
	wiz5.print();
	wiz6.print();


	cin.ignore();
	cin.get();
	return 0;
}

My output is a mess. Missing letters and ridiculous numbers. What did I do wrong? Also, can someone show me the proper syntax for reading/writing strings?
Thanks.

[AbstractionAnon]

Lets take "Gandalf" as an example:
Line 65: size() of "Gandalf" is 7 characters. This is going to write exactly 7 characters to outFile. No traling null will be written.

Now, lets look at line 130. wiz4 was constructed using default constructor. so wiz4.mName is empty.

Line 74: You're reading for mName.size() bytes, except mName is empty, so this is 0. So you haven't moved past the start of the file. Next you try and read ints, and you're reading ASCII characters.

Your choices are
1) Ensure you write a terminating null, then use getline to read until a null.
2) Write/read mName as a fixed width field.

[Konstantin2]

1. Using text mode is simplier:

void Wizard::save(ofstream& outFile)
{
outFile << mName << ' ' << mHitPoints << ' ' << mMagicPoints << ' ' << mArmor << '\n';
}

void Wizard::load(ifstream& inFile)
{
/inFile >> mName >> mHitPoints >> mMagicPoints >> mArmor;
}

2. If you really want use binary mode...

void Wizard::save(ofstream& outFile)
{
	outFile.write(mName.c_str(), mName.size() + 1);
	outFile.write((char*)&mHitPoints, sizeof(int));
	outFile.write((char*)&mMagicPoints, sizeof(int));
	outFile.write((char*)&mArmor, sizeof(int));
}

void Wizard::load(ifstream& inFile)
{
	getline(inFile, mName, '\0');
	inFile.read((char*)&mHitPoints, sizeof(int)); // read hit points
	inFile.read((char*)&mMagicPoints, sizeof(int)); // read magic points
	inFile.read((char*)&mArmor, sizeof(int)); // read armor
}

Description:
In your text
inFile.read((char*)&mName, mName.size())
program asks system to read mName.size() bytes.
mName = "Default".
mName.size()==7.
So your program asks the system to read 7 bytes even if the string in file is shorter or longer.
When actual string is longer, system reads 7 bytes into mName, and other bytes of the string will be read from file later into mHitPoints and so on. Values of mHitPoints in file will be read into other valiables. That is mistake.
One of solutions is to use special character '\0' (zero byte) to mark the end of a string. This is common practice. In this case you should write not mName.size() but mName.size()+1 bytes. Also write not mName.data() but mName.c_str(). Zero byte is added in the end of the string by c_str().
Or you can write zero byte manually by outFile << '\0';
In read process you should ask the system to read all bytes until zero byte. This is done by
getline(inFile, mName, '\0');

[hornet07]

Line 65: I thought c_str automatically produced a null-terminated c-string. I changed it to outFile.write(mName.c_str(), mName.size() + 1); to accomodate that extra '\0'.

Line 130: I don't understand the issue here. They're supposed to be blank for loading.

Line 74: I knew the size argument wasn't correct, but I have no idea how to give it the exact number it needs, other than counting the characters myself of course. How can I automatically set the right number for a string?

[hornet07]

You're a lifesaver Konstantin2 :D
The original code was actually in text. I was supposed to make it work for binary. Seeing you solve this with getline made me facepalm...For some reason I never considered taking the whole line for name.