This is why I hate File I/O

Started by
11 comments, last by Ekim_Gram 18 years, 10 months ago
Read my post below. Well, I've been working on and off with my Tetris clone and the game itself is done. All I've got left to do is the score board and and credits, so I decided to work on the socre board. My first step was to create a console version of the score board. I read this article on File I/O in C++. Using that article as a guideline, and a little bit of C (seeing how some things I wanted I knew how to do in C and not C++) I came up with this mess of a code:

#include <fstream>
#include <iostream>
#include <stdio.h>
#include <string>
using namespace std;

struct HighScore
{
	int Rank;
	int Score;
	char OutputString[10];
};

void WriteHighScoreToFile(HighScore Score);

int main()
{
	HighScore myScores[10];
	for (int i=1; i<11; i++)
	{
		myScores[i-1].Rank = i;
		myScores[i-1].Score = 0;
		WriteHighScoreToFile(myScores[i-1]);
	}
	HighScore myScores2[10];
	ifstream fin("binoutput.dat", ios::binary);
	for (int i=1; i<11; i++)
	{
		fin.read((char*)(&myScores2[i-1]), sizeof(HighScore));
		cout << "MyScore[" << i << "]" << endl;
		cout << myScores2[i-1].Rank << endl;
		cout << myScores2[i-1].Score << endl;
		puts(myScores2[i-1].OutputString);
		cout << "\n\n";
	}
	return 0;
}

void WriteHighScoreToFile(HighScore score)
{
	ofstream fout;
	fout.open("binoutput.dat", ios::app, ios::binary);
	char ScoreString[10];
	sprintf(ScoreString, "%i", score.Score);
	int stringLength = strlen(ScoreString);
	stringLength = 9 - stringLength;
	char Score[10];
	strcpy(Score, "0");
	for (int i=0; i<stringLength-1; i++)
		strcat(Score, "0");
	char FinalScore[10];
	strcpy(FinalScore, Score);
	strcat(FinalScore, ScoreString);
	strcpy(score.OutputString, FinalScore);
	fout.write((char*)(&score), sizeof(score));
	fout.close();
}


What I want it to do is basically reset the score board. Everything works fine until it get's to rank #10. Here's my output:

MyScore[1]
1
0
000000000


MyScore[2]
2
0
000000000


MyScore[3]
3
0
000000000


MyScore[4]
4
0
000000000


MyScore[5]
5
0
000000000


MyScore[6]
6
0
000000000


MyScore[7]
7
0
000000000


MyScore[8]
8
0
000000000


MyScore[9]
9
0
000000000


MyScore[10]
2573
0
As you can see, it's rank and OutputString are just out of whack. Help is greatly appreciated. [Edited by - Ekim_Gram on June 11, 2005 7:11:12 PM]
Advertisement
The output you showed us starts with MyScore[1]. If this isn't a typo, and this is real output, then I think it partly explains your problem. Arrays start at [0] remember.

You have MyScore[10]. Therefore, you'll be starting at MyScore[0], and finishing at MyScore[9]. I think MyScore[10] is generating crap because it technically doesn't exist.

I hope that helps, other than that I'm really not too sure.

ukdeveloper.
Quote:Original post by ukdeveloper
The output you showed us starts with MyScore[1]. If this isn't a typo, and this is real output, then I think it partly explains your problem. Arrays start at [0] remember.

You have MyScore[10]. Therefore, you'll be starting at MyScore[0], and finishing at MyScore[9]. I think MyScore[10] is generating crap because it technically doesn't exist.

I hope that helps, other than that I'm really not too sure.

ukdeveloper.


I thought that at first, but if you look at his loop, he's indexing by i-1 so he is treating it as a 0-based array. A little odd, but it will work.

I think I may know what it is..
No, he seems to be correcting for that in his loop indices. Anyway, the presented code is seriously convoluted. For one thing, it doesn't even make much sense to include the OutputString as part of the HighScore data - but I'll go with it for now. For another, it includes <string> and then doesn't actually USE it. Instead, there is some very strange manipulation involved in creating an OutputString. Oh, and even for binary data, just writing out a struct in one go is a pretty dangerous way of doing things.

#include <fstream>#include <iostream>#include <string>#include <sstream>using namespace std;// First I'll define some useful utility functions for the binary file IO// You can "put these in your library" and forget about it :/template <typename T>T binaryGet(istream& is) {  T result;  is.read(reinterpret_cast<char*>(&result), sizeof(T));  return result;}// and a specialization for std::strings// This is probably not exception-safe :(template<>string binaryGet<std::string>(istream& is) {  int lengthcount = binaryGet<int>(is);  // We won't be writing the null terminator to file...  char* buffer = new char[lengthcount+1];  is.read(buffer, lengthcount);  buffer[lengthcount] = '\0';  string result(buffer);  delete buffer;  return result;}// Similarly for outputtemplate <typename T>void binaryPut(T data, istream& is) {  is.write(reinterpret_cast<char*>(&data), sizeof(T));}// and a specialization for std::strings// This is probably not exception-safe :(template<>void binaryPut<string>(T data, istream& is) {  binaryPut<int>(data.length(), is);  binaryPut<int>(data.data(), is);}// And the usual string conversion method...template <typename T>string ToString(const T& input) {  stringstream ss; ss << input; return ss.str();}// Now the rest of the code is much nicer:struct HighScore {  int Rank;  int Score;  string ScoreString; // use the string dammit! It's a real textual type.  // I'll implement reading and writing as member functions instead:  void WriteTo(ostream& target) {    binaryPut<int>(Rank, target);    binaryPut<int>(Score, target);    binaryPut<string>(ScoreString, target);  }  HighScore(istream& dataSource) : Rank(binaryGet<int>(dataSource)),                                   Score(binaryGet<int>(dataSource)),                                   ScoreString(binaryGet<string>(dataSource)) {}  // And another constructor, that will make sure the ScoreString matches the  // Score  HighScore(int Rank, int Score) : Rank(Rank), Score(Score),                                    ScoreString(ToString<int>(Score)) {}};int main() {  const int TableSize = 10;  HighScore myScores[TableSize];  ofstream fout("binoutput.dat", ios::app | ios::binary);  for (int i=0; i<TableSize; i++) {    myScores[TableSize] = HighScore(i+1, 0);    myScores[TableSize].WriteTo(fout);  }  fout.close(); // <-- not having this could have been a problem before?  HighScore myScores2[TableSize];  ifstream fin("binoutput.dat", ios::binary);  for (int i=0; i<TableSize; i++) {    myScores[TableSize] = HighScore(fin);    // Do the outputting.    cout << "MyScore[" << i + 1 << "]" << endl;    cout << myScores2.Rank << endl         << myScores2.Score << endl         << myScores2.ScoreString << endl << endl;    // We could have made that a member function too :/  }  return 0;}


Where to begin...

Firstly, your entire OutputString generation can be shortened to two lines:
memset(score.OutputString, 0, sizeof(char)*10);_snprintf(score.OutputString,9,"%09d",score.Score);


Looking at the output file in a hex editor, I see that it's outputting the number 10 as 0x0D0A0000, which is what's giving you the funky value. I have no idea why. I blame ofstream.

I also note that if I run the app twice, it's just going to append to the end of the score file, instead of overwriting it.

For the sake of it, I converted the code to use C file IO routines, and it works perfectly.
struct HighScore{	int Rank;	int Score;	char OutputString[10];};void WriteHighScoreToFile(HighScore &score, FILE *pFile){		   memset(score.OutputString, 0, sizeof(char)*10);   _snprintf(score.OutputString,9,"%09d",score.Score);   fwrite(&score, sizeof(HighScore), 1, pFile);}int _tmain(int argc, _TCHAR* argv[]){      FILE *pFile = fopen("binoutput.dat","wb");      HighScore myScores[10];   for (int i=0; i<10; i++)   {      myScores.Rank = i+1;      myScores.Score = 0;      WriteHighScoreToFile(myScores, pFile);   }   fclose(pFile);      HighScore myScores2[10];   pFile = fopen("binoutput.dat","rb");   for (int i=0; i<10; i++)   {      fread(&myScores2,sizeof(HighScore),1,pFile);   	      cout << "MyScore[" << i+1 << "]" << endl;		cout << myScores2.Rank << endl;		cout << myScores2.Score << endl;		puts(myScores2.OutputString);		cout << "\n\n";	}   fclose(pFile);   printf("\r\nPress [Enter] to exit...\r\n");   getchar();   return 0;}
Quote:Original post by pragma Fury
Looking at the output file in a hex editor, I see that it's outputting the number 10 as 0x0D0A0000, which is what's giving you the funky value. I have no idea why. I blame ofstream.


Aha.

ASCII 10 is a return. The two bytes 0x0D0A are a two-byte return character encoding - that is, the newline is being converted to a DOS-style file ending, which in turn indicates that the file is being opened in text mode rather than binary.

I draw attention to this line:

fout.open("binoutput.dat", ios::app, ios::binary);

Frankly, I'm surprised that compiles - fstream::open() is supposed to take exactly two arguments, and there are three here.

Clearly what is meant is ios::app|ios::binary, that is, a bitwise OR of the flags (used to combine them).
Quote:Original post by Zahlman
Quote:Original post by pragma Fury
Looking at the output file in a hex editor, I see that it's outputting the number 10 as 0x0D0A0000, which is what's giving you the funky value. I have no idea why. I blame ofstream.


Aha.

ASCII 10 is a return. The two bytes 0x0D0A are a two-byte return character encoding - that is, the newline is being converted to a DOS-style file ending, which in turn indicates that the file is being opened in text mode rather than binary.

I draw attention to this line:

fout.open("binoutput.dat", ios::app, ios::binary);

Frankly, I'm surprised that compiles - fstream::open() is supposed to take exactly two arguments, and there are three here.

Clearly what is meant is ios::app|ios::binary, that is, a bitwise OR of the flags (used to combine them).


open does have a 3-param version, "void open(const char*, int=ios::out, int=filebuf::openprot)"

Alright, after a few days of not being home and doing nothing, I cleaned up my code a little bit. I decided having the 0's before the raw score was too much of a hassle.

As I said, everything is cleaned up and readable but I'm still having a little problem. Here's my source:

#include <fstream>#include <iostream>#include <stdio.h>using namespace std;struct SHigh_Score{	int Rank;	int Score;};SHigh_Score High_Scores[10];bool GetHighScores(){	std::ifstream fin("highscores.dat", std::ios::binary);	for (int i=0; i<10; i++)		fin.read((char*)(&High_Scores), sizeof(SHigh_Score));	return false;}bool WriteHighScoreToFile(SHigh_Score High_Score){	std::ofstream fout;	fout.open("highscores.dat",std::ios::binary);	if (fout == NULL)		return 1;	fout.write((char*)(&High_Score), sizeof(SHigh_Score));	fout.close();	return 0;}void ResetScoreBoard(){	for (int i=1; i<11; i++)	{		High_Scores[i-1].Rank = i;		High_Scores[i-1].Score = 0;		WriteHighScoreToFile(High_Scores[i-1]);	}}int main(){	ResetScoreBoard();	GetHighScores();	ifstream fin("highscores.dat", ios::binary);	for (int i=0; i<10; i++)	{		fin.read((char*)(&High_Scores), sizeof(SHigh_Score));		cout << "--High Score[" << i << "]--" << endl << endl;		cout << "Rank: \t " << High_Scores.Rank << endl;		cout << "Score: \t " << High_Scores.Score << endl;		cout << "\n\n";	}	return 0;}


And here's my output:

--High Score[0]--Rank:    10Score:   0--High Score[1]--Rank:    2Score:   0--High Score[2]--Rank:    3Score:   0--High Score[3]--Rank:    4Score:   0--High Score[4]--Rank:    5Score:   0--High Score[5]--Rank:    6Score:   0--High Score[6]--Rank:    7Score:   0--High Score[7]--Rank:    8Score:   0--High Score[8]--Rank:    9Score:   0--High Score[9]--Rank:    10Score:   0


As you can see from High Score[0]:

--High Score[0]--Rank:    10Score:   0


The rank is 10 for some reason when it should be 1. I'm a bit befuddled about this.
Quote:Original post by Ekim_Gram
As you can see from High Score[0]:

--High Score[0]--Rank:    10Score:   0



That's due to the fact that you are always overwriting your high score file with each entry.

In your WriteHighScoreToFile function, you need to make the code fout.open("highscores.dat",std::ios::binary); say fout.open("highscores.dat",std::ios::binary | std::ios::app); so it will append the scores rather than over write them. That should fix it. Good luck!

***Note that when you re-save all the scores at the end, you will need to open with just std::ios::binary so it will overwrite the old contents.
Quote:Original post by Drew_Benton
That's due to the fact that you are always overwriting your high score file with each entry.

In your WriteHighScoreToFile function, you need to make the code fout.open("highscores.dat",std::ios::binary); say fout.open("highscores.dat",std::ios::binary | std::ios::app); so it will append the scores rather than over write them. That should fix it. Good luck!

***Note that when you re-save all the scores at the end, you will need to open with just std::ios::binary so it will overwrite the old contents.


I'm still a little confused to what you're saying. Why would I want to append to the file? It'll just get gradually larger and larger every time I write to it, which is why I set it to just over write.

This topic is closed to new replies.

Advertisement