-1

I’m currently doing Knight’s Travails project. In this project you need to find the shortest way from A to B for the chess knight.

I don’t know why my program crashes when it comes to breadth-first search function. I cannot catch it with debugger because VScode freezes at reading variable “root” inside knight_moves.

Could you help me find the ussue?

I’ve created the board. It has links to every cell of the board according position of the cell. I’ve created links between cells with add_edges function. Links are possible ways to move.

So far I’ve got the code below

class Node
  attr_reader :pos
  attr_accessor :children, :search_info
  def initialize (row, column)
    @pos = [row, column]
    @children = nil
    @search_info = Hash.new
  end
end

class Board
  attr_reader :show
  def initialize
    create_board
  end


  def create_board
    board = []
    8.times do |x|
      board<<[x]
    end
    board.each_with_index do |item, index|
      8.times do |x|
      board[index] << x unless x == index
      end
    end
    board.each do |x|
      x.sort!
    end
    @board = board
  end

  def show
    @board
  end

  def fill_with_nodes
    @board.each_with_index do |item, index|
      item.map! {|column| Node.new(index,column)}
    end
  end

  def add_edges
    @board.each_with_index do |row, index|
      row.each do |node|
        node.children = []

        node.children = node.children << @board[node.pos[0]-2][node.pos[1]-1] if (0..7).include?(node.pos[0]-2) && (0..7).include?(node.pos[1]-1)
        node.children = node.children << @board[node.pos[0]-2][node.pos[1]+1] if (0..7).include?(node.pos[0]-2) && (0..7).include?(node.pos[1]+1)

        node.children = node.children << @board[node.pos[0]+2][node.pos[1]-1] if (0..7).include?(node.pos[0]+2) && (0..7).include?(node.pos[1]-1)
        node.children = node.children << @board[node.pos[0]+2][node.pos[1]+1] if (0..7).include?(node.pos[0]+2) && (0..7).include?(node.pos[1]+1)

        node.children = node.children << @board[node.pos[0]-1][node.pos[1]-2] if (0..7).include?(node.pos[0]-1) && (0..7).include?(node.pos[1]-2)
        node.children = node.children << @board[node.pos[0]+1][node.pos[1]-2] if (0..7).include?(node.pos[0]+1) && (0..7).include?(node.pos[1]-2)

        node.children = node.children << @board[node.pos[0]-1][node.pos[1]+2] if (0..7).include?(node.pos[0]-1) && (0..7).include?(node.pos[1]+2)
        node.children = node.children << @board[node.pos[0]+1][node.pos[1]+2] if (0..7).include?(node.pos[0]+1) && (0..7).include?(node.pos[1]+2)

      end
    end
  end

  def cell (row, column)
    @board[row][column]
  end

  def knight_moves (start, finish)
    raise StandardError.new("Invalid start") unless (0..7).include?(start[0]) || (0..7).include?(start[1])
    raise StandardError.new("Invalid finish") unless (0..7).include?(finish[0]) || (0..7).include?(finish[1])


    queue = []

    root = @board[finish[0]][finish[1]]
    root.search_info[:distanse] = 0
    queue << root
    until queue.empty?
      node = queue.shift
      break if node.pos == [start[0],start[1]]
      node.children.each do |child|
        unless child.search_info[:distanse] 
        child.search_info[:distanse] = node.search_info[:distanse] + 1
        child.search_info[:predecessor] = node
        queue << child
        end
      end
    end
  end

end 


#This part is for testing
puts a = Board.new
puts a.show.to_s
a.fill_with_nodes
puts a.show.to_s
a.add_edges
a.knight_moves([0,0], [0,1])
def show_cell(board,row, column)
  puts ""
  puts board.cell(row,column).pos.to_s, board.cell(row,column).children.map {|child| child.pos}.to_s ,board.cell(row,column).search_info.to_s
end
show_cell(a,2,2)

Edit: I've found that line "child.search_info[:predecessor] = node" crashes the programm. And if I use @variable to store "predecessor" instead of hash the programm runs. I don't know why though. What's the reason?

Infectos
  • 1
  • 2
  • My understanding is that the [Knight’s Travails problem](http://rubyquiz.com/quiz27.html) is to find a shortest path from a given starting square to a given ending square, without landing on any of the given *forbidden* squares. Is that the problem you are solving? If so, you have not seem to have accounted for the forbidden squares. – Cary Swoveland Jan 25 '20 at 00:29
  • I've created add_edges function, which links cells. So in #move_knigth you can move only to "children" cells of current cell. Not to every cell. As I found, my algorithm works ONLY IF I change this line (child.search_info[:predecessor] = node) to this line (child.search_info[:predecessor] = node.pos), OR I need to create new @variable and store "predecessor" in that variable. At the moment question is "why ruby crashes when I try to store instance of Node class in the hash" – Infectos Jan 25 '20 at 11:57
  • It looks like you have a memory leak caused by runaway recursion, but that should not crash Ruby. In fact, *nothing* in your code should ever crash Ruby. (Unless you write native extensions for Ruby, that is.) I can't reproduce your crash (I get an exception instead). As a general rule, if Ruby crashes, the problem is not in your code, it is in Ruby. – Jörg W Mittag Jan 25 '20 at 14:57

1 Answers1

0

As for me, the main issue with the code is its unnecessary ("incidental") complexity.

Yes, the task you're solving can be reduced to a graph traversal problem, but it doesn't mean you must represent the graph explicitly. For this particular task - where all the possible moves from the arbitrary cell are well-defined and the board itself is limited - you can easily calculate the graph edges on the fly (and without all this additional machinery that makes your code so hard to reason about - even for you). Explicit representation of the board looks redundant too (again, for this particular task).

Taking all this into account, the solution might be as simple as:

class Knight
  def initialize
    @knight_moves = [[-2, -1], [-2, 1], [-1, -2], [-1, 2], [1, -2], [1, 2], [2, -1], [2, 1]]
  end

  def move(start, stop)
    visited = {}
    queue = [[stop, nil]]

    while queue.any?
      current_cell, next_cell = queue.shift
      next if visited.has_key?(current_cell)

      visited[current_cell] = next_cell

      return build_path(start, stop, visited) if current_cell == start

      possible_moves(current_cell).each do |next_move|
        queue << [next_move, current_cell] unless visited.has_key?(next_move)
      end
    end
  end

  private

  def possible_moves(cell)
    @knight_moves.
      map { |x, y| [cell.first + x, cell.last + y] }.
      select(&method(:valid_move?))
  end

  def build_path(start, stop, visited)
    path = [start]

    while next_cell = visited[path.last]
      path << next_cell
    end

    path.last == stop ? path : nil
  end

  def valid_move?(cell)
    cell.all? { |n| n >= 0 && n <= 7 }
  end
end

knight = Knight.new
knight.move [0,0], [0,1] #=> [[0, 0], [2, 1], [1, 3], [0, 1]]
Konstantin Strukov
  • 2,899
  • 1
  • 10
  • 14