The move()
function basically does 3 main things:
- receives the number on the tile to be move.
- validates the move.
- if the move is valid, it makes it and returns
true
. Otherwise, it returns false
.
First, receiving the number on the tile to be moved involves some linear search to get the row and the column of that tile in the board.
Second, validating the move has to do with checking the adjacent tiles. Given a tile that's in row, col
in the board, these are the 4 possible adjacent tiles
row - 1, col
row, col - 1 row, col row, col + 1
row + 1, col
Notice, however, that 4 adjacent tiles is actually the maximum number of adjacent tiles for any tile to have. Meanwhile, there are tiles that have less number of adjacent tiles (e.g., the corner tiles).
Lastly, if the blank tile is in one of these adjacent locations, you should swap the two tiles. Don't forget to update the row and the column of the blank tile and return true
.
If, however, the number that was passed to our function as the number on the tile to be moved is not really in our board, or the blank tile is not in one of the adjacent positions to that tile, the move()
function should return false
immediately.
What's wrong with your code?
Well, you don't really need to use all of these variables. i
and j
which I assume they correspond to row
and col
are probably enough and can be used directly in the process of checking the adjacent squares.
I'm really confused following all of these variables as they make your code harder to read and to understand specially when there are no comments. Besides, they make your program take more memory than it'd really need with a better solution. Hopefully, I could figure out that they're used to prevent checking illegal array indexes.
Obviously, the linear search part is missing, so I'll assume you already found the tile at board[i][j]
.
So with all of these variables, you're over-complicating things a little bit. One of the main issue is, as noticed @AirThomas mentioning something about it, the if/else if/else
statements.
In short, only the first met condition of these is executed and the others are ignored. Not as you might have thought that multiple of them might be executed.
Following that, you may return true
immediately after making the move.
How to solve that?
First, minimize your usage of variables and get rid of the extra variables and simplify things a little. Try to count on two main variables i, j
or row, col
whatever you call them as they're enough for the checking adjacent tiles process. For example, I could have something like this to check the adjacent squares
// up
if (row > 0)
{
if (board[row - 1][column] is the blank tile)
{
make the move
return true
}
}
// right
if (column < d - 1)
{
if (board[row][column + 1] is the blank tile)
{
make the move
return true
}
}
// bottom
if (row < d - 1)
{
if (board[row + 1][column] is the blank tile)
{
make the move
return true
}
}
// left
if (column > 0)
{
if (board[row][column - 1] is the blank tile)
{
make the move
return true
}
}
return false
Remember why I didn't use else if
here? To allow one or more of these conditions to be met. And notice that I'm returning true
immediately once I'm making the move.
And lastly, I'm returning false
if none of these conditions is met indicating that the move is invalid. Recall that false
should be returned in case the tile wasn't found in the board at the first place.
Edit:
to find the row and the column of the tile using linear search, you may do the following
declare a bool named found and set it to false
for (row = 0 to (row < d), increment row by 1)
{
for (column = 0 to column < d, increment column)
{
if (board[row][column] is equal to tile)
{
set found to true
break
}
}
if tile found
break
}