My C issues

[Joanne]

I love that it's gone from my simple game of 15 code to the stock exchange!! Hahahahah!!

[jwinters]

Might want to read up as I already phrased that above As I said financial things are nothing to do with this post, and you'd still use Decimal at 128bit for them.

 

Not sure what you mean by a Decimal - that isn't a C type. The choice is between various sizes of int and two sizes of float. Both can be used to store decimal numbers - and indeed binary numbers or hexadecimal numbers. The difference between decimal, hexadecimal and binary is merely one of presentation. Yes, financial calculations have nothing to do with this post, but it's a good example of where people use floats when they shouldn't.

 

Another good example of where you shouldn't use floats/doubles is in a calculation like Joanne's.

 

floats/doubles are really very specialist items and you practically never use them for ordinary programming. Certainly not when solving a problem like the one presented in this thread.

 

And none of that post changes the fact that even a simple divide calculation would need a type other than Int

 

One very often does division using ints. Obviously, you need to be clear on what you're trying to achieve, and be aware of the rules of the language for how the answer will be rounded.

 

Here's an example which I wrote recently for interfacing with iSAMS. The iSAMS system uses a non-standard week numbering system for deciding which week of the year it is so you can't use the standard cweek() function. The calculation which is required is:

 

(date.yday - date.wday + 12) / 7

 

and yes, that is integer division and it very deliberately rounds the answer. This is a very similar example to what Jo's trying to do, because it's a question of calculating the index into an array.

 

Anyway, don't want to de-rail Jo's thread anymore than it is

 

Steve

 

I'm not sure that the thread has been de-railed. Jo asked for programming help and some very useful and relevant help has been given. It's good to be warned off using floats inappropriately before you get bitten by them.

 

Cheers,

John

[Joanne]

Thanks for the warning John. I was thinking out loud when suggesting the floats / doubles. I think my problem was a misplaced curly bracket. Currently I'm wondering if this is going to do what I want it to/...

 

    //set blankTile with 0 location
   blankTile[row][col] = (grid[row][col] == 0);

 

I want to save the location of my 0. So if my 0 was at grid[2][2] would it set blankTile[2][2] ?

[jwinters]

Thanks for the warning John. I was thinking out loud when suggesting the floats / doubles. I think my problem was a misplaced curly bracket. Currently I'm wondering if this is going to do what I want it to/...

 

    //set blankTile with 0 location
   blankTile[row][col] = (grid[row][col] == 0);

 

I want to save the location of my 0. So if my 0 was at grid[2][2] would it set blankTile[2][2] ?

 

Assuming that's inside a loop like this:

 

for (row = 0; row < SIZE; row++)
{
 for (col = 0; col < SIZE; col++)
 {
   blankTile[row][col] = (grid[row][col] == 0);
 }
}

 

then yes, that's going to put values into a second 2D array such that all of them are false, except for the one corresponding to the blank cell, which will be true. However I'm not sure that helps you if what you're trying to do is get the coordinates of the blank cell. You now need to do another search of your new array looking for the "true" value.

 

Assuming I've guessed your objective correctly, how about this?

 

for (row = 0; row < SIZE; row++)
{
 for (col = 0; col < SIZE; col++)
 {
   if (grid[row][col] == 0)
   {
     wanted_row = row;
     wanted_col = col;
   }
 }
}

 

It would be nicer if C included a primitive like break which would break you out of both loops, but unfortunately it doesn't. Given the small size of the array you're dealing with, it's not too bad to let the loops run to completion.

 

Hope this helps.

 

John

Edited October 17, 2016 by jwinters

[Joanne]

My spec of the problem says that the location should be saved so you don't have to search every time. So you just do one search and then when you tell the program to swap tiles, it saves the new location of the blank tile.

[jwinters]

My spec of the problem says that the location should be saved so you don't have to search every time. So you just do one search and then when you tell the program to swap tiles, it saves the new location of the blank tile.

 

But surely you don't need a whole second array in which to store said location? A couple of variables would do. E.g. rename my wanted_row and wanted_col from above as:

 

int blank_cell_row;

int blank_cell_col;

 

initialise them as previously discussed, and then just update them each time the blank cell changes. That way you have immediate knowledge of where the blank cell is all the time.

 

Cheers,

John

[Joanne]

Right I've written the swap function, but I get 'illegal move' every time I try a number to swap...

 

/**
* If tile borders empty space, moves tile and returns true, else
* returns false. 
*/
bool move(int tile)
{
int row, col, blankTile, blankTileRow, blankTileCol;
int grid[d][d];

   
   blankTileRow = (d - 1);
   blankTileCol = (d - 1);
   
   //make sure provided tile number is legit
   //if (tile > ((d*d)-1))
   //{
   //return false;
   //}
   //if (tile <= 0)
   //{
   //return false;
   //}
   //search for tile
   for (col = 0; col < d; col++)
   {
      for (row = 0; row < d; row++)
      {
      if (tile == grid[row][col])
      {
       blankTile = 0;
          //check it is next to 0
          if (((blankTileRow == (row - 1)) && (col == blankTileCol)) || (( blankTileRow == (row +1)) && (col == blankTileCol)) || 
          ((row == blankTileRow) && (blankTileCol == (col - 1))) || ((row == blankTileRow) && (blankTileCol == (col + 1))))
           {
           //swap locations of tile and 0
               grid[blankTileRow][blankTileCol] = tile;
               grid[row][col] = blankTile;
               blankTileRow = row;
               blankTileCol = col;
               
               return true;
           }
      }
      }
   }
   return false;
}

 

I kinda think it is returning false no matter what I put in, and I think it's because of that there return false at the bottom. I tried whacking an else return false in there, but it didn't like it. I'm on the edge with this now!

[jwinters]

Hi Joanne,

 

I hope you mean you're on the edge of success, and not on the edge of giving up.

 

Looking at your code I can't see immediately why it's not giving you the results which you expect, but it shouldn't be too far from being right.

 

My first suggestion would be to add some debug logging code inside the function. Like this:

 

[color=#333333]  for (col = 0; col < d; col+)
 {
   printf("Working on column %d\n", col);
   ...
[/color]

 

A few of those would let you see exactly how your logic is working out. Check exactly which row and column you expect it to succeed on, and then check whether it does.

 

HTH

John

[Joanne]

It all seems to make sense...

 

 

- - - Updated - - -

 

WAIT! I put it on the draw function.... 1 sec!

[Joanne]

So this flashes up when I select tile 4. It SHOULD swap it with the blank tile...

 

 

- - - Updated - - -

 

It should find the 4 value in column 3, row 2. But doesn't.

[mavhc]

First I'd indent it better, every time you start a { you should indent a level, I used notepad++, textfx, edit, reindent c++ code.

 

/**
* If tile borders empty space, moves tile and returns true, else
* returns false. 
*/
bool move(int tile)
{
int row, col, blankTile, blankTileRow, blankTileCol;
int grid[d][d];


blankTileRow = (d - 1);
blankTileCol = (d - 1);

//make sure provided tile number is legit
//if (tile > ((d*d)-1))
//{
//return false;
//}
//if (tile <= 0)
//{
//return false;
//}
//search for tile
for (col = 0; col < d; col++)
{
	for (row = 0; row < d; row++)
	{
		if (tile == grid[row][col])
		{
			blankTile = 0;
			//check it is next to 0
			col = 1, row = 2
			bcol = 2, brow = 2
			if (((blankTileRow == (row - 1)) && (col == blankTileCol)) || (( blankTileRow == (row +1)) && (col == blankTileCol)) || 
					((row == blankTileRow) && (blankTileCol == (col - 1))) || ((row == blankTileRow) && (blankTileCol == (col + 1))))
			{
				//swap locations of tile and 0
				grid[blankTileRow][blankTileCol] = tile;
				grid[row][col] = blankTile;
				blankTileRow = row;
				blankTileCol = col;
				
				return true;
			}
		}
	}
}
return false;
}

 

I kinda think it is returning false no matter what I put in, and I think it's because of that there return false at the bottom. I tried whacking an else return false in there, but it didn't like it. I'm on the edge with this now!

The idea is it returns true if the if is true, if so it never gets to the return false.

 

Looks like it should work the first time you call it, but each time you call it you're setting blankTileRow and blankTileCol back to their initial values. Your variables blankTileRow and blankTileCol need to be a) global, OK for simple problems, but not good practise, b) created by looking at the data, just search for the tile that's 0, or c) passed as parameters.

 

© is faster, but you're relying on the rest of the code to track the blank cell properly, (b) is slower (by microseconds of course) but means the function doesn't rely on any other code.

[Steve21]

Not sure how you're using the code in the main function anymore, but did you mean to redeclare the array?

 

bool move(int tile)
{
int row, col, blankTile, blankTileRow, blankTileCol;
[color="#FF0000"]int grid[d][d];[/color]

   
   blankTileRow = (d - 1);
   blankTileCol = (d - 1);
   

   for (col = 0; col < d; col++)
   {
      for (row = 0; row < d; row++)
      {
      if (tile == [color="#FF0000"]grid[row][col][/color])

 

What's the rest of the code as not sure it'd like but basically you have nothing in that array

 

Steve

[Joanne]

@mavhc tried making the blankTileRow and blankTileCol values global, but it didn't work. Still tells me illegal move.

 

- - - Updated - - -

 

OK @Steve21 - this is the full thing. I basically have to write the functions.

 

/**
* fifteen.c
*
* Computer Science 50
* Problem Set 3
*
* Implements Game of Fifteen (generalized to d x d).
*
* Usage: fifteen d
*
* whereby the board's dimensions are to be d x d,
* where d must be in [DIM_MIN,DIM_MAX]
*
* Note that usleep is obsolete, but it offers more granularity than
* sleep and is simpler to use than nanosleep; `man usleep` for more.
*/

#define _XOPEN_SOURCE 500

#include 
#include 
#include 
#include 

// constants
#define DIM_MIN 3
#define DIM_MAX 9

// board
int board[DIM_MAX][DIM_MAX];

// dimensions
int d, blankTileCol, blankTileRow;

// prototypes
void clear(void);
void greet(void);
void init(void);
void draw(void);
bool move(int tile);
bool won(void);

int main(int argc, string argv[])
{
   // ensure proper usage
   if (argc != 2)
   {
       printf("Usage: fifteen d\n");
       return 1;
   }

   // ensure valid dimensions
   d = atoi(argv[1]);
   if (d < DIM_MIN || d > DIM_MAX)
   {
       printf("Board must be between %i x %i and %i x %i, inclusive.\n",
           DIM_MIN, DIM_MIN, DIM_MAX, DIM_MAX);
       return 2;
   }

   // open log
   FILE* file = fopen("log.txt", "w");
   if (file == NULL)
   {
       return 3;
   }

   // greet user with instructions
   greet();

   // initialize the board
   init();
   //Set blank tile location
   blankTileRow = (d - 1);
   blankTileCol = (d - 1);

   // accept moves until game is won
   while (true)
   {
       // clear the screen
       clear();

       // draw the current state of the board
       draw();

       // log the current state of the board (for testing)
       for (int i = 0; i < d; i++)
       {
           for (int j = 0; j < d; j++)
           {
               fprintf(file, "%i", board[i][j]);
               if (j < d - 1)
               {
                   fprintf(file, "|");
               }
           }
           fprintf(file, "\n");
       }
       fflush(file);

       // check for win
       if (won())
       {
           printf("ftw!\n");
           break;
       }

       // prompt for move
       printf("Tile to move: ");
       int tile = GetInt();
       
       // quit if user inputs 0 (for testing)
       if (tile == 0)
       {
           break;
       }

       // log move (for testing)
       fprintf(file, "%i\n", tile);
       fflush(file);

       // move if possible, else report illegality
       if (!move(tile))
       {
           printf("\nIllegal move.\n");
           usleep(500000);
       }

       // sleep thread for animation's sake
       usleep(500000);
   }
   
   // close log
   fclose(file);

   // success
   return 0;
}

/**
* Clears screen using ANSI escape sequences.
*/
void clear(void)
{
   printf("\033[2J");
   printf("\033[%d;%dH", 0, 0);
}

/**
* Greets player.
*/
void greet(void)
{
   clear();
   printf("WELCOME TO GAME OF FIFTEEN\n");
   usleep(2000000);
}

/**
* Initializes the game's board with tiles numbered 1 through d*d - 1
* (i.e., fills 2D array with values but does not actually print them).  
*/
void init(void)
{
   // grid size
   int grid[d][d];
   int row, col, val;
   
   val = 1;
   //populate grid by row, move along columns
   for (col = 0; col < d; col++)
   {
      for (row = 0; row < d; row++)
       {
       grid[row][col] = (d * d) - val;
       val = val + 1;
       }
   }
       grid[d-1][d-1] = 0;
       
   //switch 1 and 2 for odd number of tiles
       if ((d %2 == 0) && grid[row][col] == 2)
       {
       grid[row][col] = 1;
       }
       else 
           if ((d %2 == 0) && grid[row][col] == 1)
           {
               grid[row][col] = 2;
           }
}

/**
* Prints the board in its current state.
*/
void draw(void)
{
int grid[d][d];
   int row, col;
   int val;
   row = 0;
   val = 1;
   //populate grid by row, move along columns

   for (col = 0; col < d; col++)
   {
       
      for (row = 0; row < d; row++)
       {
       
       grid[row][col] = (d * d) - val;
       val = val + 1;
       
       if ((d %2 == 0) && grid[row][col] == 2)
       {
       grid[row][col] = 1;
       }
       else 
           if ((d %2 == 0) && grid[row][col] == 1)
           {
               grid[row][col] = 2;
           }
       if (grid[row][col] == 0)
       {
           printf("  _");
       }
       else 
       {
       printf(" %2d", grid[row][col]);
       } 
       
       }
       printf("\n");
   }
}

/**
* If tile borders empty space, moves tile and returns true, else
* returns false. 
*/
bool move(int tile)
{
int row, col, blankTile;
int grid[d][d];

   //make sure provided tile number is legit
   //if (tile > ((d*d)-1))
   //{
   //return false;
   //}
   //if (tile <= 0)
   //{
   //return false;
   //}
   //search for tile
   for (col = 0; col < d; col++)
   {printf("Working on column %d\n", col);
      for (row = 0; row < d; row++)
      {    printf("Working on row %d\n", row);
      if (tile == grid[row][col])
      {
       blankTile = 0;
          //check it is next to 0
          if (((blankTileRow == (row - 1)) && (col == blankTileCol)) || (( blankTileRow == (row +1)) && (col == blankTileCol)) || 
          ((row == blankTileRow) && (blankTileCol == (col - 1))) || ((row == blankTileRow) && (blankTileCol == (col + 1))))
           {
           //swap locations of tile and 0
               grid[blankTileRow][blankTileCol] = tile;
               grid[row][col] = blankTile;
               blankTileRow = row;
               blankTileCol = col;
               
               return true;
           }
      }
      }
   }
   return false;
}

/**
* Returns true if game is won (i.e., board is in winning configuration), 
* else false.
*/
bool won(void)
{
int counter, col, row;
int grid[d][d];
//create counter
counter = 0;
       for (col = 0; col < d; col++)
           {
           for (row = 0; row < d; row++)
               {
               counter = counter + 1;
               //check all numbers are correct
               if (grid[row][col] != counter)
                   {
                   return false;
                   }
               }
           }
   return true;
}

[mavhc]

OK, so it's a global/local variable issue, does https://www.tutorialspoint.com/cprogramming/c_scope_rules.htm make sense?

 

https://www.tutorialspoint.com/cprogramming/c_scope_rules.htm

 

// board

int board[DIM_MAX][DIM_MAX];

 

// dimensions

int d, blankTileCol, blankTileRow;

 

means those 4 variables are global, as they're not inside a function, so you can just reference them in any function.

 

You have a board array, and then you have a grid array, not sure why.

[Joanne]

Ooops, I've been using grid and the board one was pre-written. Perhaps I need to change over to board!

 

Yes the global and local makes perfect sense. I wondered why I had to keep repeating declarations! Think I need to do some re-arranging then. I tested the functions separately, and they always worked on their own... d'oh.

[Steve21]

Aye, you're effectively making a new board per function. So it'll work fine when you add the numbers in.

 

The second time you're using it it's another blank board, so will pull random parts of data from memory and give odd things, example below running once to add to board, then another again to move:

 

As above you'll see the same table, but as it's two different declarations it's got different values.

 

As mav said above, declare the table globally then you don't want to do it per function

 

Steve

[Joanne]

Right swapped all that up a bit and I'm still getting illegal move!

 

/**
* fifteen.c
*
* Computer Science 50
* Problem Set 3
*
* Implements Game of Fifteen (generalized to d x d).
*
* Usage: fifteen d
*
* whereby the board's dimensions are to be d x d,
* where d must be in [DIM_MIN,DIM_MAX]
*
* Note that usleep is obsolete, but it offers more granularity than
* sleep and is simpler to use than nanosleep; `man usleep` for more.
*/

#define _XOPEN_SOURCE 500

#include 
#include 
#include 
#include 

// constants
#define DIM_MIN 3
#define DIM_MAX 9

// board
int board[DIM_MAX][DIM_MAX];

// dimensions
int d;

//global declarations
int row, col, val, blankTile, blankTileCol, blankTileRow;

// prototypes
void clear(void);
void greet(void);
void init(void);
void draw(void);
bool move(int tile);
bool won(void);

int main(int argc, string argv[])
{
   // ensure proper usage
   if (argc != 2)
   {
       printf("Usage: fifteen d\n");
       return 1;
   }

   // ensure valid dimensions
   d = atoi(argv[1]);
   if (d < DIM_MIN || d > DIM_MAX)
   {
       printf("Board must be between %i x %i and %i x %i, inclusive.\n",
           DIM_MIN, DIM_MIN, DIM_MAX, DIM_MAX);
       return 2;
   }
   
   //Set blank tile location
   blankTileRow = (d - 1);
   blankTileCol = (d - 1);
   blankTile = board[blankTileRow][blankTileCol];

   // open log
   FILE* file = fopen("log.txt", "w");
   if (file == NULL)
   {
       return 3;
   }

   // greet user with instructions
   greet();

   // initialize the board
   init();


   // accept moves until game is won
   while (true)
   {
       // clear the screen
       clear();

       // draw the current state of the board
       draw();

       // log the current state of the board (for testing)
       for (int i = 0; i < d; i++)
       {
           for (int j = 0; j < d; j++)
           {
               fprintf(file, "%i", board[i][j]);
               if (j < d - 1)
               {
                   fprintf(file, "|");
               }
           }
           fprintf(file, "\n");
       }
       fflush(file);

       // check for win
       if (won())
       {
           printf("ftw!\n");
           break;
       }

       // prompt for move
       printf("Tile to move: ");
       int tile = GetInt();
       
       // quit if user inputs 0 (for testing)
       if (tile == 0)
       {
           break;
       }

       // log move (for testing)
       fprintf(file, "%i\n", tile);
       fflush(file);

       // move if possible, else report illegality
       if (!move(tile))
       {
           printf("\nIllegal move.\n");
           usleep(500000);
       }

       // sleep thread for animation's sake
       usleep(500000);
   }
   
   // close log
   fclose(file);

   // success
   return 0;
}

/**
* Clears screen using ANSI escape sequences.
*/
void clear(void)
{
   printf("\033[2J");
   printf("\033[%d;%dH", 0, 0);
}

/**
* Greets player.
*/
void greet(void)
{
   clear();
   printf("WELCOME TO GAME OF FIFTEEN\n");
   usleep(2000000);
}

/**
* Initializes the game's board with tiles numbered 1 through d*d - 1
* (i.e., fills 2D array with values but does not actually print them).  
*/
void init(void)
{
   val = 1;
   //populate board by row, move along columns
   for (col = 0; col < d; col++)
   {
      for (row = 0; row < d; row++)
       {
       board[row][col] = (d * d) - val;
       val = val + 1;
       }
   }
       board[d-1][d-1] = 0;
       
   //switch 1 and 2 for odd number of tiles
       if ((d %2 == 0) && board[row][col] == 2)
       {
       board[row][col] = 1;
       }
       else 
           if ((d %2 == 0) && board[row][col] == 1)
           {
               board[row][col] = 2;
           }
}

/**
* Prints the board in its current state.
*/
void draw(void)
{

   val = 1;
   //populate board by row, move along columns

   for (col = 0; col < d; col++)
   {
       
      for (row = 0; row < d; row++)
       {
       
       board[row][col] = (d * d) - val;
       val = val + 1;
       
       if ((d %2 == 0) && board[row][col] == 2)
       {
       board[row][col] = 1;
       }
       else 
           if ((d %2 == 0) && board[row][col] == 1)
           {
               board[row][col] = 2;
           }
       if (board[row][col] == 0)
       {
           printf("  _");
       }
       else 
       {
       printf(" %2d", board[row][col]);
       } 
       
       }
       printf("\n");
   }
}

/**
* If tile borders empty space, moves tile and returns true, else
* returns false. 
*/
bool move(int tile)
{


   //make sure provided tile number is legit
   //if (tile > ((d*d)-1))
   //{
   //return false;
   //}
   //if (tile <= 0)
   //{
   //return false;
   //}
   //search for tile
   for (col = 0; col < d; col++)
   {printf("Working on column %d\n", col);
      for (row = 0; row < d; row++)
      {    printf("Working on row %d\n", row);
      if (tile == board[row][col])
      {
       blankTile = 0;
          //check it is next to 0
          if (((blankTileRow == (row - 1)) && (col == blankTileCol)) || (( blankTileRow == (row +1)) && (col == blankTileCol)) || 
          ((row == blankTileRow) && (blankTileCol == (col - 1))) || ((row == blankTileRow) && (blankTileCol == (col + 1))))
           {
           //swap locations of tile and 0
               board[blankTileRow][blankTileCol] = tile;
               board[row][col] = blankTile;
               blankTileRow = row;
               blankTileCol = col;
               
               return true;
           }
      }
      }
   }
   return false;
}

/**
* Returns true if game is won (i.e., board is in winning configuration), 
* else false.
*/
bool won(void)
{
int counter;

//create counter
counter = 0;
       for (col = 0; col < d; col++)
           {
           for (row = 0; row < d; row++)
               {
               counter = counter + 1;
               //check all numbers are correct
               if (board[row][col] != counter)
                   {
                   return false;
                   }
               }
           }
   return true;
}

[Joanne]

So... going off my 'check50' results

 

I think the problem may lie with my init function....

[jwinters]

I'd add some more debugging printfs. Straight after the printf("Working on row %d\n", row);

 

I'd add:

 

printf ("Tile is %d\n", tile);
printf ("Value at (%d, %d) is %d\n", row, col, board[row][col]);

 

and keep drilling in like that until you find out which of your assumptions is wrong.

 

Cheers,

John

[Steve21]

I think the problem may lie with my init function....

 

Aye there is. You're looping the wrong way around.

 

You're going for 1 column, loop across rows.

 

So Col1 -> Row 1/2/3, Col2 -> Row 1/2/3

 

Which is why the log shows your table like this:

 

8|5|2
7|4|1
6|3|0

 

You need to change your loop so it's doing Row1 -> Col 1/2/3 etc, so Rows and then Col inside it. Not the other way around.

 

Steve

[Steve21]

Yep both the Init ones need some loop changes, the draw one isn't really doing anything as you don't need to re-make the array you just need to draw it

 

And your "swap 1/2" function isn't working at all currently.

 

After those though seems to work fine (Not sure how much of the answer you want, so shout if you want specific changes)

 

 

Steve

[Joanne]

I've changed the loop and they both initialize properly now. So I need to sort my draw out I guess!

[Steve21]

I've changed the loop and they both initialize properly now. So I need to sort my draw out I guess!

 

Think of it this way, all you're doing is reading the value in the array and printing it out. No need to write anything into it.

 

Steve

[Joanne]

deleted the two lines at the beginning and OMG IT WORKS!

[Steve21]

deleted the two lines at the beginning and OMG IT WORKS!

 

Woot woot

 

One thing to check, does your 1/2 swap work on your version now? As wasn't working when I tried it so changed few bits for that.

 

Steve