Breadth first search implementation stuck in infinite loop

Viewed 111

I'm trying to implement a breadth first search to solve a maze, but I keep getting stuck in an infinite loop and I have no idea why this is happening. Rows and columns are the number of rows and columns for my maze/grid. I am starting from the top left cell, and checking to see if a neighbour is within the grid before adding them to the queue.

export function elementInGrid<T>(neighbour: number[], rows: number, columns: number): boolean{
    const [i, j] = neighbour;
    return (0 <= i && i <= rows - 1) && (0 <= j && j <= columns - 1);
}

function arrayEquality<T>(a: T[], b: T[]): boolean{
    return a.sort().toString() === b.sort().toString()
}

export const breadthFirstSearch = (rows: number, columns: number) => {
    const startPoint = [0, 0];
    const endPoint = [rows - 1, columns - 1];
    const queue = [startPoint];
    while (queue.length > 0){
        const [i, j] = queue.shift()!;
        if (arrayEquality([i, j], endPoint)){
            break
        }
        const neighbours = [[i - 1, j], [i, j + 1], [i + 1, j], [i, j - 1]]
        neighbours.forEach(neighbour => {
            if (elementInGrid(neighbour, rows, columns)){
                queue.push(neighbour);
            }
        })     
    }
}
2 Answers

Your algorithm assumes there are no cycles in the graph, which doesn't seem like it'd be the case given your neighbor list. The algorithm moves from cell 0, 0 to 0, 1 which has 0, 0 as a neighbor and gets enqueued. Then, 0, 0 has 0, 1 as a neighbor which has 0, 0 as a neighbor....

It's unusual that you're ignoring coordinate ordering when checking whether you reached the destination but that's unrelated to the infinite loop (arrayEquality is misleadingly named--it only checks primitives reliably due to stringification and ignores ordering).

From a design standpoint, I wouldn't hardcode the start and endpoints into the grid. I'm not sure it makes much sense to have every cell connected in the grid, either, so likely you'd want to pass in a graph with walls (i.e. neighbors are specific to each cell, assuming this is really a maze).

Also, the algorithm does nothing when it finds a path through the grid. I've taken the liberty to adjust a few of these things to make the example a bit more purposeful, omitting the maze neighboring. I'm also using JS so you can run it in a stack snippet.

const inGrid = (x, y, rows, cols) => 
  x >= 0 && y >= 0 && x < cols && y < rows
;

const shortestPath = (rows, cols, src, dst) => {
  const queue = [[src, []]];
  
  for (const visited = new Set(); queue.length;) {
    const [[x, y], path] = queue.shift();
    const key = "" + [x, y];
    
    if (visited.has(key)) continue;
    
    visited.add(key);
    path.push([x, y]);

    if (key === "" + dst) return path;

    [[x-1, y], [x, y+1], [x+1, y], [x, y-1]].forEach(e => {
      if (inGrid(...e, rows, cols)) {
        queue.push([e, path.slice()]);
      }
    });
  }
};

console.log(JSON.stringify(shortestPath(5, 5, [0, 0], [4, 4])));

Here's an example on a rather contrived maze with walls:

const shortestPath = (maze, src, dst) => {
  const queue = [[src, []]];
  
  for (const visited = new Set(); queue.length;) {
    const [[x, y], path] = queue.shift();
    const key = "" + [x, y];
    
    if (visited.has(key)) continue;
    
    visited.add(key);
    path.push([x, y]);

    if (key === "" + dst) return path;

    queue.push(...maze[key].map(e => [e, path.slice()]));
  }
};

const maze = {
  "0,0": [[0, 1], [1, 0]],
  "0,1": [[0, 0], [0, 2]],
  "0,2": [[0, 1], [1, 2]],
  "1,0": [[0, 0], [1, 1]],
  "1,1": [[1, 0]],
  "1,2": [[0, 2]]
};
/*
.--------.
         |
|  --.   |
|    |     
`--------`
*/
console.log(JSON.stringify(shortestPath(maze, [0, 0], [1, 2])));

This BFS algo goes into an infinite loop because the visited cells list are not stored anywhere. So, the algo has no clue if a cell has been explored earlier or not, which is the reason why same cells of the maze are getting visited repeatedly.
To avoid such scenario we can have the maze itself in the function and each time we visit a cell we have to mark it as visited in the maze, so that next time if we encounter it in the loop it's not picked again and thus it will help us avoid the infinite loop. We will create a maze with all elements set to 0(marks not visited) and then when we visit any cell, we are gonna set that cell value to 1. So, the next time we will ensure that we only pick those cells which are valid and not visited earlier. The following is the correct code:


    function elementInGrid(neighbour: number[], rows: number, columns: number): boolean{
        const [i, j] = neighbour;
        return (0 (a: T[], b: T[]): boolean{
        return a.sort().toString() === b.sort().toString()
    }

    const breadthFirstSearch = (rows: number, columns: number, maze: number[][]) => {
        const startPoint = [0, 0];
        const endPoint = [rows - 1, columns - 1];
        const queue = [startPoint];
        while (queue.length > 0){
            const [i, j] = queue.shift()!;
            maze[i][j] = 1
            if (arrayEquality([i, j], endPoint)){
                console.log("Reached target")
                break
            }
            const neighbours = [[i - 1, j], [i, j + 1], [i + 1, j], [i, j - 1]]
            neighbours.forEach(neighbour => {
                if (elementInGrid(neighbour, rows, columns) && (maze[neighbour[0]][neighbour[1]] == 0)){
                    console.log(neighbour)
                    queue.push(neighbour);
                    maze[neighbour[0]][neighbour[1]] = 1
                }
            })     
        }
    }
    var maze = [[0,0,0],[0,0,0],[0,0,0]]
    breadthFirstSearch(3,3,maze)

Hope that helps!!

Related