patterncppMinor
Sliding tile puzzle
Viewed 0 times
slidingpuzzletile
Problem
I'm making a sliding tile puzzle using C++, and have code that exchanges the blank tile for one adjacent to it.
Using user input, the program takes the input and puts it in the function which uses a
I was simply wondering if it's better coding to have 4 separate functions, one for each direction like this:
Or is my current code with its
Note:
```
void puzzle::swap(int d) //d where 1 = up, 2 = left, 3 = down, 4 = right
{
int temp = 0;
switch (d) //determines direction
{
case 1://swaps empty with upward tile
if (Y == 0) //checks if null is on upper border, if true returns to prevent error
{
return;
}
temp = grid[X][Y - 1]; //swapping positions
grid[X][Y - 1] = grid[X][Y];
grid[X][Y] = temp;
Y -= 1;//null tile's new position
break;
case 2://swaps empty with left tile
if (X == 0)//checks if null is on left border, if true returns to prevent error
{
return;
}
temp = grid[X - 1][Y]; //swapping positions
grid[X - 1][Y] = grid[X][Y];
grid[X][Y] = temp;
X -= 1;
break;
case 3: //swaps empty with downward tile
if (Y == 3) //checks if null is on bottom border, if true returns to prevent error
{
return;
}
temp = grid[X][Y + 1]; //swapping positions
grid[X][Y + 1] = grid[X][Y];
grid[X][Y] = temp;
Y += 1; //null tile'
Using user input, the program takes the input and puts it in the function which uses a
switch statement to determine which direction the user wants to switch, then preforms the switch if applicable.I was simply wondering if it's better coding to have 4 separate functions, one for each direction like this:
void puzzle::swapLeft()
{
if (X == 0)
{
return;
}
temp = grid[X - 1][Y]; //swapping positions
grid[X - 1][Y] = grid[X][Y];
grid[X][Y] = temp;
X -= 1;
return
}Or is my current code with its
switch statement?Note:
X is where the empty tile's X position is and Y is empty tile's Y position.```
void puzzle::swap(int d) //d where 1 = up, 2 = left, 3 = down, 4 = right
{
int temp = 0;
switch (d) //determines direction
{
case 1://swaps empty with upward tile
if (Y == 0) //checks if null is on upper border, if true returns to prevent error
{
return;
}
temp = grid[X][Y - 1]; //swapping positions
grid[X][Y - 1] = grid[X][Y];
grid[X][Y] = temp;
Y -= 1;//null tile's new position
break;
case 2://swaps empty with left tile
if (X == 0)//checks if null is on left border, if true returns to prevent error
{
return;
}
temp = grid[X - 1][Y]; //swapping positions
grid[X - 1][Y] = grid[X][Y];
grid[X][Y] = temp;
X -= 1;
break;
case 3: //swaps empty with downward tile
if (Y == 3) //checks if null is on bottom border, if true returns to prevent error
{
return;
}
temp = grid[X][Y + 1]; //swapping positions
grid[X][Y + 1] = grid[X][Y];
grid[X][Y] = temp;
Y += 1; //null tile'
Solution
self documenting code:
Identifier Names
It is traditional that Identifier names that are all caps (i.e. X and Y) are macros. So best to use mixed case or lowercase only names.
switch (d) //determines direction
{
case 1: swapUp(); break;
case 2: swapLeft(); break;
case 3: swapDown(); break;
case 4: swapRight(); break;
default:
throw 1;
}
void swapUp() {swap(0, 1, -1, 0);}
void swapLeft() {swap(1, 0, 0, -1);}
void swapDown() {swap(0, -1, -1, 3);}
void swapRight() {swap(-1, 0, 3, -1);}
void swap(int xDelta, int yDelta, int xCheck, int yCheck)
{
//swaps empty with upward tile
if ((Y == yCheck) || (X == xCheck)) //checks if null is on upper border, if true returns to prevent error
{
return;
}
temp = grid[X - xDelta][Y - yDelta]; //swapping positions
grid[X - xDelta][Y - yDelta] = grid[X][Y];
grid[X][Y] = temp;
Y = Y - yDelta;//null tile's new position
X = X - xDelta;
}Identifier Names
It is traditional that Identifier names that are all caps (i.e. X and Y) are macros. So best to use mixed case or lowercase only names.
Code Snippets
switch (d) //determines direction
{
case 1: swapUp(); break;
case 2: swapLeft(); break;
case 3: swapDown(); break;
case 4: swapRight(); break;
default:
throw 1;
}
void swapUp() {swap(0, 1, -1, 0);}
void swapLeft() {swap(1, 0, 0, -1);}
void swapDown() {swap(0, -1, -1, 3);}
void swapRight() {swap(-1, 0, 3, -1);}
void swap(int xDelta, int yDelta, int xCheck, int yCheck)
{
//swaps empty with upward tile
if ((Y == yCheck) || (X == xCheck)) //checks if null is on upper border, if true returns to prevent error
{
return;
}
temp = grid[X - xDelta][Y - yDelta]; //swapping positions
grid[X - xDelta][Y - yDelta] = grid[X][Y];
grid[X][Y] = temp;
Y = Y - yDelta;//null tile's new position
X = X - xDelta;
}Context
StackExchange Code Review Q#121852, answer score: 4
Revisions (0)
No revisions yet.