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

Optimizations to 8-puzzle

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
puzzleoptimizationsstackoverflow

Problem

I am a CS student about to enter my junior year. I am attempting to get better and better at programming and thought that this would be a good place to toss my code out there to see if some of you could give me some tips on how to make it better.

If you've got tips as to different websites I could go to for this sort of thing, please don't hesitate to tell me.

```
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Scanner;

/***
* Compilation: javac EightPuzzle.java Execution: java EightPuzzle Dependencies:
* MinPQ.java
*
* AI solution to N-by-N slider puzzle using heuristic function which is depth
* in tree plus number of tiles out of position.
*
* Note: integer 0 corresponds to blank cell
*
* Goal state is of the following form and is hardwired into the dist() and
* manhattan() methods.
*
* 1 2 3 4 5 6 7 8
*
***/

public class EightPuzzle implements Comparable {

private final static int N = 3;
private final static String[] names = { " ", " 1", " 2", " 3", " 4",
" 5", " 6", " 7", " 8" };
private static int totalEnqueued;
private final static int[][] solved = { { 1, 2, 3 }, { 4, 5, 6 },
{ 7, 8, 0 } };
private int moves;
private int[][] tiles;
private EightPuzzle parent;
private int priority;
private int distance;
int zeroLocX = 0;
int zeroLocY = 0;

// allocate separate memory for new tiles array
EightPuzzle(int[][] tiles) {
this.tiles = new int[N][N];
for (int i = 0; i priority()) //but we still need to know whether to return a -1 or 1
return -1;
return 1;
}
}
return 0; //if we make it here they are equal
} else if (b.priority() > priority()) //if the distances are different
return

Solution

In the Constructor, you manually copy an array:

for(int j = 0; j < N; j++)
{
    this.tiles[i][j] = tiles[i][j];
}


Which can be replaced with:

System.arraycopy(tiles[i], 0, this.tiles[i], 0, N);


And the entire switch in your priority() method can be replaced by:

manhatDist += posDiff(x, y, (tiles[x][y] == 0 ? 2 : (int) (tiles[x][y] / 3.5)), (tiles[x][y] + 2) % 3);


To test this code, I used:

for(int i = 0; i < 9; i++)
{
    System.out.println(i + "\t"  + (i == 0 ? 2 : (int) (i / 3.5)) + "\t" + (i + 2) % 3);
}


Which produces:

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


Which is what you're already doing manually.

Additionally, your constructor with more parameters exactly duplicates code from the smaller one, so you could replace (your newly changed!):

this.tiles = new int[N][N];
for(int i = 0; i < N; i++)
{
    System.arraycopy(tiles[i], 0, this.tiles[i], 0, N);
}


With:

this(new int[N][N]);


So the smaller constructor does that part, then continues with the additional options.

Code Snippets

for(int j = 0; j < N; j++)
{
    this.tiles[i][j] = tiles[i][j];
}
System.arraycopy(tiles[i], 0, this.tiles[i], 0, N);
manhatDist += posDiff(x, y, (tiles[x][y] == 0 ? 2 : (int) (tiles[x][y] / 3.5)), (tiles[x][y] + 2) % 3);
for(int i = 0; i < 9; i++)
{
    System.out.println(i + "\t"  + (i == 0 ? 2 : (int) (i / 3.5)) + "\t" + (i + 2) % 3);
}
0    2    2
1    0    0
2    0    1
3    0    2
4    1    0
5    1    1
6    1    2
7    2    0
8    2    1

Context

StackExchange Code Review Q#19644, answer score: 3

Revisions (0)

No revisions yet.