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

Shortest path through a maze

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

Problem

I was trying to solve this problem in Java:


Given a 2-D array of black and white entries representing a maze with designated entrance and exit points, find the shortest path from entrance to exit, if one exists. The black entry represents a wall and the white entry represents an open space.

I tried to solve it using a variant of the Breadth-First-Search algorithm, where from a starting position, I examine all its possible adjacent positions. If the adjacent spot has not been visited(a Map containing the spaces that have been visited) or is not in the in-process queue, I add it to the in-process queue. If I encounter an adjacent spot that has been visited, I examine its 'weight', if the weight of the adjacent spot is less than one added to the current weight of the space in process, I update the weight of the visited node. I keep on doing this until I encounter the destination or all the nodes in the maze have been processed.

I have crafted my algorithm as follows, it will be great if I could get some feedback on refining it further.

```
import java.util.Set;
import java.util.List;
import java.util.Queue;
import java.util.Map;
import java.util.HashSet;
import java.util.ArrayList;
import java.util.LinkedList;
import java.util.HashMap;

public class ShortestMazePath{
class Node{
private final int x;
private final int y;
private final int weight;
private final Node previous;
Node(int x,int y,Node previous,int weight){
this.x = x;
this.y = y;
this.previous = previous;
this.weight = weight;
}
public int getX(){
return this.x;
}
public int getY(){
return this.y;
}
public int getWeight(){
return this.weight;
}
public Node getPrevious(){
return this.previous;
}
@Override public boolean equals(Object o){
if(o == null){
return f

Solution

-
It looks like there is a bug in this piece of code:

List neighbors = new ArrayList(){{
             add(new Node(currentX-1,currentY,current,currentWeight));
             add(new Node(currentX+1,currentY,current,currentWeight));
             add(new Node(currentX,currentY+1,current,currentWeight));
             add(new Node(currentX,currentY-1,current,currentWeight));}};


The weight of the neighbors should be currentWeight + 1, not currentWeight(because we need one more step to reach the neighbor from the current node). And I would call it distance, not weight.

-
The updatedNode method is redundant. Nodes are never updated in a breadth-first search. You can get rid of it.

-
Map visited = new HashMap(); A map that maps a node to itself doesn't make much sense. I would use a Set here. And I do not see the point of having a Set inProcess. The entire algorithm is implemented in a pretty strange way. Here is pseudo code of a standard BFS implementation:

discovered = an empty set
queue = an empty queue
startVetrex.dist = 0
queue.add(startVertex)
discovered.add(startVertex)
while not queue.isEmpty():
    v = queue.poll()
    for neighbor <- neighbors(v):
        if not discovered.contains(neighbor):
            neighbor.dist = v.dist + 1
            neighbor.parent = v
            discovered.add(neighbor)
            queue.add(neighbor)


That's it. No need to update vertices or having several sets(visited, inQueue and so on).

-
Whitespaces: there should be whitespaces around binary operators, before and after curly brackets, after the for, while and if keywords, between method parameters. For instance,

private Set getNeighbors(Node current,int m,int n,boolean[][] maze){


should be

private Set getNeighbors(Node current, int m, int n, boolean[][] maze) {


and

while((current = inProcess.poll())!= null){


should be

while ((current = inProcess.poll()) != null) {


-
Blank lines: it is conventional to have a blank line between methods, constructors and so on. Here is a refined part of your Node class:

class Node {
    private final int x;
    private final int y;
    private final int weight;
    private final Node previous;

    Node(int x, int y, Node previous, int weight) {
        this.x = x;
        this.y = y;
        this.previous = previous;
        this.weight = weight;
    }

    public int getX(){
        return this.x;
    }

    public int getY(){
        return this.y;
    }

    ...
}


-
You should also write documentation comments for all public classes and methods.

Code Snippets

List<Node> neighbors = new ArrayList<Node>(){{
             add(new Node(currentX-1,currentY,current,currentWeight));
             add(new Node(currentX+1,currentY,current,currentWeight));
             add(new Node(currentX,currentY+1,current,currentWeight));
             add(new Node(currentX,currentY-1,current,currentWeight));}};
discovered = an empty set
queue = an empty queue
startVetrex.dist = 0
queue.add(startVertex)
discovered.add(startVertex)
while not queue.isEmpty():
    v = queue.poll()
    for neighbor <- neighbors(v):
        if not discovered.contains(neighbor):
            neighbor.dist = v.dist + 1
            neighbor.parent = v
            discovered.add(neighbor)
            queue.add(neighbor)
private Set<Node> getNeighbors(Node current,int m,int n,boolean[][] maze){
private Set<Node> getNeighbors(Node current, int m, int n, boolean[][] maze) {
while((current = inProcess.poll())!= null){

Context

StackExchange Code Review Q#81802, answer score: 2

Revisions (0)

No revisions yet.