0

I've managed to come up with a working BFS algorithm that returns the entire traversal path, however I'm unsure of how to get the shortest path.

This is my code:

public ArrayList<Node> get_neighbours(Node node, Grid grid) {

        ArrayList<Node> neighbours = new ArrayList<>();

        int R = grid.getRl();
        int C = grid.getCl();

        int[] dr = new int[]{-1, +1, 0, 0};
        int[] dc = new int[]{0, 0, +1, -1};

        for (int i=0; i<4; i++) {
            int rr = node.getR() + dr[i];
            int cc = node.getC() + dc[i];

            if (rr < 0 || cc < 0) continue;
            if (rr >= R || cc >= C) continue;

            if (grid.getNodes()[rr][cc].getVal() == "V") continue;
            if (grid.getNodes()[rr][cc].getVal() == "X") continue;

            Node neighbour = grid.getNodes()[rr][cc];
            neighbours.add(neighbour);
        }

        return neighbours;

    }

    public Queue<Node> find_path_bfs(Node start, Node end, Grid grid) {

        Queue<Node> queue = new LinkedList<>();
        Queue<Node> path = new LinkedList<>();

        queue.add(start);
        grid.mark_visited(start.getR(), start.getC());

        while (!queue.isEmpty()) {
            Node node = queue.remove();
            path.add(node);

            if (node == end) break;

            ArrayList<Node> adj_nodes = get_neighbours(node, grid);
            for (Node n : adj_nodes) {
                if (!(n.getVal() == "V")) {
                    queue.add(n);
                    n.setVal("V");
                }
            }
        }
        return path;
    }

I took a look at this post, that suggests saving the path to the current node as well, but I'm not sure how to implement this in Java as I would have to have a queue that stores both a Node and a List of Nodes together as one item. My Java is a little rusty, so I'm not sure if this is possible or if there is a better way.

  • If you are really doing a BFS, why do you think it wouldn't return the shortest path? – Daniel Apr 09 '20 at 03:10
  • If you want to efficiently find an optimal path, you're better off using Uniform Cost Search or A*. Uniform Cost Search involves assigning a weight to each edge between nodes and using a priority queue instead of a queue. A* is the same as UCS, except it also involves using a heuristic function to guide the search. Both of these options will guarantee finding the optimal path, and A* will do it in the shortest amount of time – awarrier99 Apr 09 '20 at 03:12
  • Or use [Dijkstra's algorithm](https://en.wikipedia.org/wiki/Dijkstra%27s_algorithm#Algorithm). – Andreas Apr 09 '20 at 03:21
  • @Daniel my solution returns a list of every node traversed during the search, I was hoping to have it just return the shortest path instead. – Jordanhenry Apr 09 '20 at 03:23
  • @awarrier99 I'm using this a learning exercise/side project more than anything. I plan to implement multiple path-finding algorithms, I just happened to choose this one first – Jordanhenry Apr 09 '20 at 03:25
  • @Jordanhenry gotcha well I think your issue is that you're building your path as you go. Usually with search algorithms you find the shortest path, and create some sort of method of storing a reference for a node's parent node (usually using a hashmap or something similar of ChildNode -> ParentNode) and then you reconstruct your path by traversing this data structure once you've reached your goal node. Does that make sense? – awarrier99 Apr 09 '20 at 03:27
  • @Jordanhenry I added the gist of it in an answer – awarrier99 Apr 09 '20 at 03:38
  • A BFS is a big `while(queue has nodes)`. Theoretically, it shouldn't return anything and should be easy to stop when you step into the node you're looking for. – Daniel Apr 09 '20 at 03:42
  • @Daniel if you're using BFS, or any search algorithm for that matter, to find a path between two nodes, why would this ever not return something? – awarrier99 Apr 09 '20 at 03:45
  • @Daniel also notice that OP breaks out of the while loop upon reaching the `end` node – awarrier99 Apr 09 '20 at 03:47

1 Answers1

2

Your issue is that right now you're using the Queue<Node> path variable as a visited list, rather than a path list. Instead of building the path as you go, store references to each child node's parent and then traverse back through this. Something like this:

public ArrayList<Node> find_path_bfs(Node start, Node end, Grid grid) {

    Queue<Node> queue = new LinkedList<>();
    List<Node> path = new ArrayList<>();

    queue.add(start);
    grid.mark_visited(start.getR(), start.getC());

    Map<Node, Node> parentMap = new HashMap<>();

    Node node = start;
    parentMap.put(start, null); // start node has no parent, so we end path reconstruction here
    while (!queue.isEmpty()) {
        node = queue.remove();
        // path.add(node);

        if (node == end) break;

        ArrayList<Node> adj_nodes = get_neighbours(node, grid);
        for (Node n : adj_nodes) {
            if (!(n.getVal() == "V")) {
                queue.add(n);
                parentMap.put(n, node); // mark current node as parent to neighbor
                n.setVal("V");
            }
        }
    }

    Node curr = node;
    while (curr != null) {
        path.add(0, curr);
        curr = parentMap.get(curr);
    }

    return path;
}

I changed path to an ArrayList so that you can insert at the beginning (since you're reconstructing the path from the last node, rather than the first one). Alternatively, you could keep it as a Queue and reverse the order of the elements.

awarrier99
  • 3,628
  • 1
  • 12
  • 19