Sign in to follow this  
always_learning

don't understand basic file I/O

Recommended Posts

Hi, I'm really new to file I/O and don't know why the following code doesn't work:
// load maps
void LoadMaps()
{
	// this will store the file name
	char map_id[9];
	
	// this guy is an array of files, each one is a map
	FILE* maps[MAXNUMLEVELS];

	// run through a for loop and open all map files
	for (int h = 0; h < MAXNUMLEVELS; h++)
	{
		sprintf(map_id, "map0%d.txt", h);
		maps[h] = fopen(map_id, "r");
	}	
	
	// make sure map file isn't NULL
	if (maps[0] == NULL)
	{
		char* error_message = "Error: Unable to open map files.";
		WriteToLogFile(error_message);
	}
	
	// TODO: Load map data
	for (int p = 0; p < MAXNUMLEVELS; p++)
	{
		for (int m = 0; m < WORLDSIZE_X; m++)
		{
			for (int n = 0; n < WORLDSIZE_Y; n++)
			{
				g_levels[p].map[n][m] = fgetc(maps[p]);
				
			}
		}
	}

	// maps have been loaded so close the files now
	for (int k = 0; k < MAXNUMLEVELS; k++)
	{
		fclose(maps[k]);
	}
}	

Here is the map file:
111111111111111111
111111111111111111
111111111111111111
111111111111111111
111111111111111111
111111111111111111
111111111111111111
111111111111111111
111111111111111111
111111111111111111
111111111111111111
111111111111111111

Share this post


Link to post
Share on other sites
Oluseyi    2103
First, what do you mean by "doesn't work"? What are you expecting and what are you experiencing?

Second, when using C-style strings (ie, null-terminated character arrays), you need to allocate one extra char for the null. map05.txt is 9 characters; you need 10 to represent it as a C-string. If your MAXNUMLEVELS is 2 or 3 digits, then you need even more space... Failing to format your string properly will break many other routines, so that may be the first of your problems.

Third, why do you open all the files in one loop, read from them in another, and then close them in a third loop? Why not, in a single loop, open a file, read from it, and close it? There is a limit to the number of file descriptors a process may hold; I'm hoping you're not hitting it...



For us to really help you, tell us what you want to do.

Share this post


Link to post
Share on other sites
Quote:
Original post by Oluseyi
First, what do you mean by "doesn't work"? What are you expecting and what are you experiencing?

Second, when using C-style strings (ie, null-terminated character arrays), you need to allocate one extra char for the null. map05.txt is 9 characters; you need 10 to represent it as a C-string. If your MAXNUMLEVELS is 2 or 3 digits, then you need even more space... Failing to format your string properly will break many other routines, so that may be the first of your problems.

Third, why do you open all the files in one loop, read from them in another, and then close them in a third loop? Why not, in a single loop, open a file, read from it, and close it? There is a limit to the number of file descriptors a process may hold; I'm hoping you're not hitting it...



For us to really help you, tell us what you want to do.


I am expecting my function that draws the tiles to work, but it doesn't draw anything. The screen remains blank. MAXNUMLEVELS is 4. I may bump it up to a double digits number but for now I'm leaving it at 4.

And regarding the loops I'm kinda rusty on C so I guess that was a simple oversight (wrong word you know what I mean).

Also I changed the 9 to 10 and that didn't change anything.

Share this post


Link to post
Share on other sites
I think the problem is that I don't really understand how fgetc() works.

I did a quick google search and found this:

fgetc() reads the next character from stream and returns
it as an unsigned char cast to an int, or EOF on end of
file or error.

So when it reads each '1' is fgetc() returning the char or the letter or something else? Not sure what I mean, hopefully you know what I mean though.

Share this post


Link to post
Share on other sites
Hmm well I fixed it with some tinkering.

Namely:

g_levels[p].map[n][m] = fgetc(maps[p]);


to

g_levels[p].map[n][m] = fgetc(maps[p]) - 48;


Since the decimal value of the character '1' is 48.

Share this post


Link to post
Share on other sites
Krohm    5030
Then just write '1'. It's more expressive.
Sure, everybody could figure out in a few moments 48 could be related to a special char you somehow decided to use as a reference but making it explicit isn't a bad idea.

Share this post


Link to post
Share on other sites
Rasmadrak    196
You can use the integer value of a letter or number by using 'x' ... i:e


int number;
number = '1'; // <-- notice the ' '
number = 48;

Share this post


Link to post
Share on other sites
Quote:
Original post by Oluseyi
What happens when you need more than 10 tile types?


Is there a way to read a file with just numbers, all separated by a space?

like:

1 4 4 4 5 10 11 2 2
0 5 6 2 2 10 1 1 1

etc...

Share this post


Link to post
Share on other sites
TheTroll    883
[opinion]
The way you are doing this is very dangerous. You should never assume that your data is going to be valid and in the format that you are expecting. Also your way does not allow for different sized maps.

So how I would suggest doing it is, for each line of the map have it terminated with a character (/0) would be good for this. So you read each line until you reach the terminating character. You continue to read lines until you reach the EOF (end of file).

A couple simple while loops and you are set. Since this looks like standard C it means we don't have access to std::vector to store the maps. So you either need to have a maximum size allowed, or you a linked list for storing the map info. In this case I would go with a linked list.
[/opinion]

theTroll

Share this post


Link to post
Share on other sites
ok ty

one last question

say I wanted to read the file number by number instead of one character at a time
and say each number is separated by a space, like so:

1 1 0 9 2 10 15 1 2 3 3
1 1 0 9 2 10 15 1 2 3 3
7 1 0 9 2 10 15 1 2 3 3
1 5 0 9 2 10 15 1 2 3 3

how would i read that into a 2 dimensional array?
like in semi psedoc-code
i dont really understand file i/o

Share this post


Link to post
Share on other sites
MHOOO    148
If you know how many Numbers there are per "line" of your file, then you can use fscanf (or preferably fscanf_s if available).
http://www.cplusplus.com/reference/clibrary/cstdio/fscanf.html

Basically you'd have something like this:

#include <stdio.h>
int number;
int row = 0;
// lets assume myFileHandle is a working handle to a file

// this while loop will run until the end of file has been reached
while(!feof(myFileHandle)) {
for(int column=0; column<11; ++column) { // 11 numbers per line
fscanf(myFileHandle,"%d",&number);

// at this point, <number> will have the number at the <row>th row and <column>th column.
// all you need to do now, is put those values inside an array.
// Unfortunately I have no clue as to what would be an appropriate way to do that in standard C - possibly something like a linked list using structs.
// In C++ you'd simply use std::vector ...

}
++row;
}

Share this post


Link to post
Share on other sites
Zahlman    1682
0) Like Oluseyi said right at the beginning: Handle each file separately. Notice that, as it stands, you only check for successful opening of the first file. That doesn't imply anything about any of the others. If you do it all in one loop, you won't risk running out of file descriptors, and you also won't have to keep around that ugly array of them.

1) FFS. It's '\0', not '/0', you guys. Also, it's a bad idea to expect the user to put weird characters like that into a text file. Return characters (i.e., end-of-line) work perfectly well as delimiters.

2) Don't write comments to say what things do, when it's already perfectly obvious what they do. The person reading your code should already be expected to know what a for-loop does, and is expecting a function named "LoadMaps" to load maps. When you comment these things, it makes the reader pay attention to things that are not important. This is bad, because it plants the seed of paranoia.

Comments are used to explain how you are doing what you are doing.

BTW: The most common reader of your code, by far, is you. Don't annoy yourself.

3) Try cutting things up into helper functions to make the process more clear.

4) There is no need to assign a bit of text to a variable just to pass it to a function call. You can pass any expression as a parameter to a function (unless you are e.g. passing by non-const reference).


void LoadMap(const char* name, int* destination)
{
FILE* map_data = fopen(name, "r");

if (!map_data)
{
WriteToLogFile("Error: Unable to open map file.");
}

for (int m = 0; m < WORLDSIZE_X; m++)
{
for (int n = 0; n < WORLDSIZE_Y; n++)
{
fscanf(map_data, "%d", &destination[n * WORLDSIZE_X + m]);
}
}

fclose(map_data);
}

void LoadMaps()
{
char map_id[16]; // let's be nice and generous ;P
// BTW, what is the leading 0 for in your file names? Did you want
// all the numbers to be a specific "width", zero-padded on the left?
// In that case, you want something like "map%02d.txt".
for (int h = 0; h < MAXNUMLEVELS; ++h)
{
sprintf(map_id, "map0%d.txt", h);
LoadMap(map_id, g_levels[h].map);
}
}

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this