patternjavaModerate
Find a path through a maze
Viewed 0 times
throughmazepathfind
Problem
This maze has multiple entry points, and quite obviously multiple exits. The entry or exit is not provided at input. Of the multiple sources and destinations, any single path from any source to any destination should be returned. The path need not be the optimal or shortest path.
I'm looking for code optimization, good practices etc. Please ignore the numeric prefixes (i.e. Coordinate3 etc.)
Two questions:
-
Please verify complexity: O(nm)
-
The enum
```
final class Coordinate3 {
private final int row;
private final int col;
public Coordinate3(int row, int col) {
this.row = row;
this.col = col;
}
public int getX() {
return row;
}
public int getY() {
return col;
}
public boolean equals(Object o) {
/**
* http://stackoverflow.com/questions/6364258/override-equals-method
* NOTE: order of this check is designed for efficiency
*/
/**
* easiest. if this results in true then we prevented a lot of hassles. comparisons can be made without any fear
* NPE or classcast
*/
if (this == o)
return true;
/**
* Mechanisms to prevent popping up exceptions. Ideally such checks better be performed at higher level.
*/
if (o == null)
return false;
if (getClass() != o.getClass())
return false;
// now to the check.
final Coordinate3 coordinate = (Coordinate3) o;
return coordinate.row == row && coordinate.col == col;
}
public int hashCode() {
return row + col;
}
@Override
public String toString() {
return row + " : " + col;
}
}
public class Maze3 {
private
I'm looking for code optimization, good practices etc. Please ignore the numeric prefixes (i.e. Coordinate3 etc.)
Two questions:
-
Please verify complexity: O(nm)
-
The enum
Direction is so called nested. I mean not a separate Java file but internal to the Maze class and it is also static. Is this the right way to do for enum? I have followed the prototype of Node class in Sun's linkedlist.java.```
final class Coordinate3 {
private final int row;
private final int col;
public Coordinate3(int row, int col) {
this.row = row;
this.col = col;
}
public int getX() {
return row;
}
public int getY() {
return col;
}
public boolean equals(Object o) {
/**
* http://stackoverflow.com/questions/6364258/override-equals-method
* NOTE: order of this check is designed for efficiency
*/
/**
* easiest. if this results in true then we prevented a lot of hassles. comparisons can be made without any fear
* NPE or classcast
*/
if (this == o)
return true;
/**
* Mechanisms to prevent popping up exceptions. Ideally such checks better be performed at higher level.
*/
if (o == null)
return false;
if (getClass() != o.getClass())
return false;
// now to the check.
final Coordinate3 coordinate = (Coordinate3) o;
return coordinate.row == row && coordinate.col == col;
}
public int hashCode() {
return row + col;
}
@Override
public String toString() {
return row + " : " + col;
}
}
public class Maze3 {
private
Solution
Here are a few comments about the style. You'll find comments about the algorithm itself at the end.
Naming
The
The same kind of things are sometimes called
(
Getters
In
Early stop
It seems like the loop in
will not do anything once
Types
It's a bit weird to compare the content of the maze to
As a drawback, it does make the definition of mazes more tedious.
The method you've defined that takes
At the end, here's what my code is like :
```
import java.util.*;
final class Coordinate {
public final int x;
public final int y;
public Coordinate(int x, int y) {
this.x = x;
this.y = y;
}
public boolean equals(Object o) {
/**
* http://stackoverflow.com/questions/6364258/override-equals-method
* NOTE: order of this check is designed for efficiency
*/
/**
* easiest. if this results in true then we prevented a lot of hassles. comparisons can be made without any fear
* NPE or classcast
*/
if (this == o)
return true;
/**
* Mechanisms to prevent popping up exceptions. Ideally such checks better be performed at higher level.
*/
if (o == null)
return false;
if (getClass() != o.getClass())
return false;
// now to the check.
final Coordinate coordinate = (Coordinate) o;
return coordinate.x == x && coordinate.y == y;
}
public int hashCode() {
return x + y;
}
@Override
public String toString() {
return x + " : " + y;
}
}
public class Maze {
private final boolean[][] maze;
public Maze(boolean[][] maze) {
if (maze.length == 0) throw new IllegalArgumentException("The maze is empty.");
this.maze = maze;
}
/**
* Returns the solution to the maze. Of the multiple sources and destinations, any single path from any source to
* any destination should be returned. The path need not be optimal / shortest.
*
* @return the set which is the solution to the maze.
*/
public Set solve() {
/ trying to find entry point in the first and last row. /
final Set mazeSolution = new LinkedHashSet();
for (int i = 0; i visitedSet) {
Coordinate coor = new Coordinate(x, y);
if (visitedSet.contains(coor)) {
return false;
}
if (isExit(x, y, visitedSet)) {
visitedSet.add(coor);
return true;
}
visitedSet.add(coor);
for (Direction dir : Direction.values()) {
int newRow = x + dir.y;
int newCol = y + dir.x;
if (isInBounds(newRow, newCol) && maze[newRow][newCol] && solvedMaze(newRow, newCol, visitedSet))
return true;
}
visitedSet.remove(coor);
return false;
}
boolean isExit(int x, int y, Set visitedSet) {
return ((x == 0) || (x == maze.length - 1) || ((y == maze[0].length - 1) || y == 0))
&& !visitedSet.isEmpty();
}
boolean isInBounds(int x, int y) {
return x >= 0 && y >= 0 && x < maze.length && y < maze[0].length;
}
private static enum Direction {
NORTH(0, -1), EAST(1, 0), SOUTH(0, 1), WEST(-1, 0);
public final int x;
public final int y;
private Direction(int x, int y) {
this.x = x;
this.y = y;
}
}
public static void main(String[] args) {
boolean[][] m1 = { { false, true, false },
Naming
The
3 at the end of the class names is a bit awkward.The same kind of things are sometimes called
x, dx or row.(
Coordinate and Direction look pretty similar too but trying to merge them showed me they were actually different in the spirit.)Getters
In
Coordinate and Direction, the members are constant (either explicitly with final or just because no method changes them). Though, you don't need to bother defining getters, just make them public and that will be it. (I am quite aware that this is not the way most Java developpers see things but at least you get a different point of view).Early stop
It seems like the loop in
for (Direction dir : Direction.values()) {
int newRow = x + dir.y;
int newCol = y + dir.x;
if (!gotMaze && isInBounds(newRow, newCol) && maze[newRow][newCol] == 1) {
gotMaze = solvedMaze(newRow, newCol, visitedSet);
}
}
if (!gotMaze) {
visitedSet.remove(coor);
}
return gotMaze;will not do anything once
gotMaze is true. Thus, this might as well just be :for (Direction dir : Direction.values()) {
int newRow = x + dir.y;
int newCol = y + dir.x;
if (isInBounds(newRow, newCol) && maze[newRow][newCol] == 1 && solvedMaze(newRow, newCol, visitedSet))
return true;
}
visitedSet.remove(coor);
return false;Types
It's a bit weird to compare the content of the maze to
1 every time. It would probably be easier to define the type of the content as a boolean.As a drawback, it does make the definition of mazes more tedious.
The method you've defined that takes
x and y as parameters should/could take coordinates.At the end, here's what my code is like :
```
import java.util.*;
final class Coordinate {
public final int x;
public final int y;
public Coordinate(int x, int y) {
this.x = x;
this.y = y;
}
public boolean equals(Object o) {
/**
* http://stackoverflow.com/questions/6364258/override-equals-method
* NOTE: order of this check is designed for efficiency
*/
/**
* easiest. if this results in true then we prevented a lot of hassles. comparisons can be made without any fear
* NPE or classcast
*/
if (this == o)
return true;
/**
* Mechanisms to prevent popping up exceptions. Ideally such checks better be performed at higher level.
*/
if (o == null)
return false;
if (getClass() != o.getClass())
return false;
// now to the check.
final Coordinate coordinate = (Coordinate) o;
return coordinate.x == x && coordinate.y == y;
}
public int hashCode() {
return x + y;
}
@Override
public String toString() {
return x + " : " + y;
}
}
public class Maze {
private final boolean[][] maze;
public Maze(boolean[][] maze) {
if (maze.length == 0) throw new IllegalArgumentException("The maze is empty.");
this.maze = maze;
}
/**
* Returns the solution to the maze. Of the multiple sources and destinations, any single path from any source to
* any destination should be returned. The path need not be optimal / shortest.
*
* @return the set which is the solution to the maze.
*/
public Set solve() {
/ trying to find entry point in the first and last row. /
final Set mazeSolution = new LinkedHashSet();
for (int i = 0; i visitedSet) {
Coordinate coor = new Coordinate(x, y);
if (visitedSet.contains(coor)) {
return false;
}
if (isExit(x, y, visitedSet)) {
visitedSet.add(coor);
return true;
}
visitedSet.add(coor);
for (Direction dir : Direction.values()) {
int newRow = x + dir.y;
int newCol = y + dir.x;
if (isInBounds(newRow, newCol) && maze[newRow][newCol] && solvedMaze(newRow, newCol, visitedSet))
return true;
}
visitedSet.remove(coor);
return false;
}
boolean isExit(int x, int y, Set visitedSet) {
return ((x == 0) || (x == maze.length - 1) || ((y == maze[0].length - 1) || y == 0))
&& !visitedSet.isEmpty();
}
boolean isInBounds(int x, int y) {
return x >= 0 && y >= 0 && x < maze.length && y < maze[0].length;
}
private static enum Direction {
NORTH(0, -1), EAST(1, 0), SOUTH(0, 1), WEST(-1, 0);
public final int x;
public final int y;
private Direction(int x, int y) {
this.x = x;
this.y = y;
}
}
public static void main(String[] args) {
boolean[][] m1 = { { false, true, false },
Code Snippets
for (Direction dir : Direction.values()) {
int newRow = x + dir.y;
int newCol = y + dir.x;
if (!gotMaze && isInBounds(newRow, newCol) && maze[newRow][newCol] == 1) {
gotMaze = solvedMaze(newRow, newCol, visitedSet);
}
}
if (!gotMaze) {
visitedSet.remove(coor);
}
return gotMaze;for (Direction dir : Direction.values()) {
int newRow = x + dir.y;
int newCol = y + dir.x;
if (isInBounds(newRow, newCol) && maze[newRow][newCol] == 1 && solvedMaze(newRow, newCol, visitedSet))
return true;
}
visitedSet.remove(coor);
return false;import java.util.*;
final class Coordinate {
public final int x;
public final int y;
public Coordinate(int x, int y) {
this.x = x;
this.y = y;
}
public boolean equals(Object o) {
/**
* http://stackoverflow.com/questions/6364258/override-equals-method
* NOTE: order of this check is designed for efficiency
*/
/**
* easiest. if this results in true then we prevented a lot of hassles. comparisons can be made without any fear
* NPE or classcast
*/
if (this == o)
return true;
/**
* Mechanisms to prevent popping up exceptions. Ideally such checks better be performed at higher level.
*/
if (o == null)
return false;
if (getClass() != o.getClass())
return false;
// now to the check.
final Coordinate coordinate = (Coordinate) o;
return coordinate.x == x && coordinate.y == y;
}
public int hashCode() {
return x + y;
}
@Override
public String toString() {
return x + " : " + y;
}
}
public class Maze {
private final boolean[][] maze;
public Maze(boolean[][] maze) {
if (maze.length == 0) throw new IllegalArgumentException("The maze is empty.");
this.maze = maze;
}
/**
* Returns the solution to the maze. Of the multiple sources and destinations, any single path from any source to
* any destination should be returned. The path need not be optimal / shortest.
*
* @return the set which is the solution to the maze.
*/
public Set<Coordinate> solve() {
/* trying to find entry point in the first and last row. */
final Set<Coordinate> mazeSolution = new LinkedHashSet<Coordinate>();
for (int i = 0; i < maze[0].length; i++) {
if (maze[0][i]) {
if (solvedMaze(0, i, mazeSolution)) {
return mazeSolution;
}
}
if (maze[maze.length - 1][i]) {
if (solvedMaze(0, i, mazeSolution)) {
return mazeSolution;
}
}
}
/* trying to find entry point in the first and last column */
for (int i = 0; i < maze.length; i++) {
if (maze[i][0]) {
if (solvedMaze(i, 0, mazeSolution)) {
System.out.println("case 3");
return mazeSolution;
}
}
if (maze[i][maze[0].length - 1]) {
if (solvedMaze(i, maze[0].length - 1, mazeSolution)) {
return mazeSolution;
}
}
}
return Collections.emptySet();
}
private boolean solvedMaze(int x, int y, Set<Coordinate> visitedSet) {
Coordinate coor = new Coordinate(x, y);
if (visitedSet.contains(coor)) {
return false;
}
Context
StackExchange Code Review Q#41133, answer score: 13
Revisions (0)
No revisions yet.