reading from a file

[momof4]

so, I'm writing a sudoku program, reading numbers from a file to pupulate my array board[r][c]. I had it working just fine, but then I was asked what would happen if the file didn't have the right number of numbers in it, so now I have to figure out how to verify that. I thought i could do it pretty easily, but when I tested it, it started adding in random numbers to fill out the array. When I changed it so it would stop at the end of the file, first it didn't work because I guess there's a space at the end of the file, so the count was off. When I changed it to just read until it stopped reading, it wasn't filling out the array correctly. How do I fix this?

bool readFile(const string fileName, int board[9][9])
{
      // open the file
   ifstream fin(fileName);
   if (fin.fail())
   {
      cout << "Unable to open file " << fileName << endl;
      return false;
   }

   // read the file into the board
   int count = 0;
   for (int r = 0; r < 9; r++)
      for (int c = 0; c < 9; c++)
         while (fin >> board[r][c])
              count++;
     
   if (count != 81)
   {
      cout << "Error: wrong amount of numbers in file. There are "
           << count << " numbers in the file.\n";
      return false;
   }

   // close file
   fin.close();

   return true;
}

[MikeyBoy]

Look at the logic of your code.

At line 15, you begin a while loop, that keeps looping as long as it's possible to keep reading numbers into board[r][c].

So, when r is 0, and c is 0, it enters the while loop, and attempts to read a number into board[0][0].

Then, it reads another number into board[0][0].

Then, it reads another number into board[0][0].

And so on, until it reaches the end of the file, and can no longer read numbers from the file, and the while loop exits.

Then, the inner for loop iterates, so that r is 0 and c is 1. We enter the while loop, and it tries to read another number from the file. However, fin is at the end of the file, so the read fails and the while loop exits, leaving board[0][1] with whatever random stuff happened to be there (because you haven't initialised board).

This then happens for every other cell on the board.

The final result is that board[0][0] contains the last number in the file, and every other element of board contains garbage from uninitialised memory.

[Handy Andy]

Hello momof4,

First off post the input file so everyone can see what yo are working with. This helps. It also helps to post the whole code because sometimes problems start in the code that is not shown. Like how is the array defined?

A question I have is the input file to contain numbers to setup a game board or does it contain all the numbers for a game or the numbers of a game played?

The for loops are designed to read 81 numbers. No more or no less. But the while loop will continue reading putting everything into one element until "eof" is reached, then the while loop fails and the for loops take over and start changing the "row" and "column" indices.

What you actually get from fin >> board[r][c] will depend on the C++ standards that you are compiling to.

There are several ways that you could change how the file is read, but first need to know what the file looks like.

You said:

it started adding in random numbers to fill out the array.

The program does not arbitrarily choose a random number to put into the file unless you tell it to by using something to generate a random number. Again this will depend on the C++ standards you are using.

Here are some suggestions for your code. These are not required, but can help.

#include <fstream>

constexpr size_t MAXROW{ 9 };
constexpr size_t MAXCOL{ 9 };

bool readFile(const std::string fileName, int board[][MAXCOL])
{
	// open the file
	std::ifstream fin(fileName);

	if (!fin)
	{
		std::cout << "Unable to open file " << fileName << '\n';
		return false;
	}

	// read the file into the board
	int count = 0;

	for (int row = 0; row < MAXROW; row++)
		for (int col = 0; col < MAXCOL; col++)
			while (fin >> board[row][col])
				count++;

	if (count != 81)
	{
		std::cout << "Error: wrong amount of numbers in file. There are "
			<< count << " numbers in the file.\n" << std::endl;

		return false;
	}

	// close file
	//fin.close(); // <--- Not necessary, but OK to leave.

	return true;
}

Lines 3 and 4 are to replace the magic numbers in the rest of the function. It does not show as well, but these are global variables with file scope so they can be used anywhere.

In line 6 when passing a 1D or 2D array to a function the first dimension's size is not required, but OK if you put one there. The size of the second dimension is required.

Line 11 can be shortened to this. It works the same or maybe a little better than what you used.

Lines 20 - 22 are changed a bit. By using "row" and "col" instead of "r" and "c" just makes the code a little easier to read and understand. Using the constants to replace the magic numbers makes the code easier to maintain or change. These constants may not be as easy to see how they work as well as it would be in a different program where they would be used more often.

Closing the file stream may be good form, but it is not required. in a function when you leave the function the stream will close. Also if you are going to use a "close" statement you should put one after your line 21 to be proper. but still not necessary to do this.

Hope that helps,

Andy

[MikeyBoy]

in a function when you leave the function the stream will close.

That's inaccurate and misleading. The file is closed when the stream object is destroyed. That's not necessarily the end of the function; it could be much sooner, when an inner scope is closed. Or, alternatively, if the stream persists past the end of the function (perhaps be being returned, or being global), the file will stay open after execution leaves the function.

[momof4]

okay, I see the problem in the logic, but I still can't figure out how to fix it!
and Andy, those suggestions may be helpful to improve the code, but it doesn't really fix the problem I'm having

[MikeyBoy]

okay, I see the problem in the logic, but I still can't figure out how to fix it!

Simple - you need to read just one number per array element, not multiple numbers.

[Handy Andy]

@MikeyBoy,

Point understood. I will work on improving this in the future. Also the way I put this was based on the code given. I was thinking for the most part it fit the code.

@momof4,

I know that the suggestions do not fix your problem, but they do help a little and in the future it should help you write better code.

The first step is to figure out what is going wrong before you can fix it.

I can offer this:

for (int row = 0; row < MAXROW; row++)
	for (int col = 0; fin >> board[row][col] && col < MAXCOL; col++)
		count++;

This is untested because I do not know what you are trying to read with your input file. Anything I might create may not match what you are using. I will work on it and give it a try.

Hope that helps,

Andy

[Handy Andy]

Hello momof4,

After some testing I found this to work:
for (int col = 0; col < MAXCOL && fin >> board[row][col]; col++)
I had the && condition reversed originally. Sorry about that.

Andy