HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Sliding tile puzzle

Submitted by: @import:stackexchange-codereview··
0
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 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:

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.