15
\$\begingroup\$

This is my first Ruby gem that claims to provide all the rules of the chess game. My goal was to keep this library as simple as possible via OOP principles, and I'd be glad to hear any feedback from you. In particular, I'm interested in code organization and OOP issues

The source code and useful README can be found at GitHub: https://github.com/anikeef/chess_engine

The library consists of 5 main parts:

  • Game class that implements the chess rules
  • Board class that provides data structure for the chess board
  • Piece class and its inheritors, which I've tried to make as lightweight as possible
  • MoveValidator module that provides validations methods to Game class
  • Move class that I've found useful to make move canceling easier. I'm particularly interested about what you think about this decision

Also it contains the CLI class for command line interface, available with chess_engine executable.

NOTE: The "bishop" piece here is referred as the "elephant" because of the russian tradition, but for the rest of the app there should be no difference in naming

Game

require_relative "board"
require_relative "move_validator"
require_relative "move"

module ChessEngine
  class InvalidMove < StandardError; end

  ##
  # This class provides all the rules for the chess game. It recognizes check,
  # checkmate and stalemate. Move validations logic can be found in the
  # MoveValidator module, which is included in this class

  class Game
    attr_accessor :name
    attr_reader :current_color
    include MoveValidator

    def initialize
      @board = Board.new
      @board.set_default
      @current_color = :white
      @last_piece = nil
      @name = nil
      @promotion_coords = false
    end

    ##
    # Accepts the move string in algebraic notation, e.g. "e2e4",
    # and applies it to the board.
    # Raises InvalidMove if:
    # * Game is already over
    # * Pawn promotion should be executed first
    # * Empty square is chosen
    # * Player tries to move piece of the opponent
    # * Move is invalid (checks via the MoveValidator module)
    # * Move is fatal (king is attacked after the move)
    #
    # After successfull move, the method changes the current player or
    # goes into "A pawn needs promotion" state, which can be checked by
    # #needs_promotion? method

    def move(string)
      from, to = Game.string_to_coords(string)
      piece = @board.at(from)
      raise InvalidMove, "Game is over" if over?
      raise InvalidMove, "#{@current_color} player should execute pawn promotion first" if needs_promotion?
      raise InvalidMove, "Empty square is chosen" if piece.nil?
      raise InvalidMove, "This is not your piece" unless piece.color == @current_color
      raise InvalidMove, "Invalid move" unless valid_moves(from).include?(to)
      move = Move.new(@board, from, to)
      move.commit
      if king_attacked?
        move.rollback
        raise InvalidMove, "Fatal move"
      end

      @last_piece = piece
      piece.moves_count += 1
      @promotion_coords = to and return if piece.pawn? && [7, 0].include?(to[1])
      next_player
    end

    ##
    # Returns a piece (or nil if the square is empty) at given coordinates
    # === Example
    #   g = Game.new
    #   g["e2"] #=> <Pawn ...>
    #   g["e4"] #=> nil

    def [](str)
      letters = ("a".."h").to_a
      return nil unless /[a-h][1-8]/.match?(str)
      @board.at([letters.find_index(str[0]), str[1].to_i - 1])
    end

    ##
    # Returns the board in the nice-looking string

    def draw
      @board.to_s
    end

    ##
    # Accepts a string with name of the piece.
    # Promotes a pawn and changes the current player.
    # Raises InvalidMove if promotion is not needed or invalid +class_name+
    # has been passed
    # === Example
    #   game.promotion("queen")

    def promotion(class_name)
      unless needs_promotion? && ["rook", "knight", "elephant", "queen"].include?(class_name.downcase)
        raise InvalidMove, "Invalid promotion"
      end
      @board.set_at(@promotion_coords, Module.const_get("ChessEngine::#{class_name.capitalize}").new(@current_color))
      @promotion_coords = nil
      next_player
    end

    ##
    # Accepts a +length+ sybmol :short or :long. Ensures that castling is
    # possible and commits appropriate moves. Otherwise, raises InvalidMove

    def castling(length)
      row = @current_color == :white ? 0 : 7
      king = @board.at([4, row])
      if length == :short
        rook = @board.at([7, row])
        line = [5, 6]
        moves = [Move.new(@board, [4, row], [6, row]),
          Move.new(@board, [7, row], [5, row])]
      else
        rook = @board.at([0, row])
        line = [1, 2, 3]
        moves = [Move.new(@board, [4, row], [2, row]),
          Move.new(@board, [0, row], [3, row])]
      end
      raise InvalidMove, "Invalid castling" unless
        king && rook && king.moves_count == 0 && rook.moves_count == 0 &&
        line.all? { |x| @board.at([x, row]).nil? }

      moves.each { |move| move.commit }
      if king_attacked?
        moves.each { |move| move.rollback }
        raise InvalidMove, "Fatal move"
      end
      @last_piece = nil
      next_player
    end

    ##
    # Returns true if game is over

    def over?
      @board.pieces_coords(@current_color).all? do |coords|
        safe_moves(coords).empty?
      end
    end

    ##
    # Checks if pawn promotion is needed

    def needs_promotion?
      !!@promotion_coords
    end

    ##
    # Returns true if current king is attacked

    def check?
      king_attacked?
    end

    private

    def king_attacked?
      king_coords = @board.king_coords(@current_color)
      [[1, 1], [-1, 1], [-1, -1], [1, -1]].each do |move|
        next_coords = relative_coords(king_coords, move)
        piece = @board.at(next_coords)
        return true if piece && piece.color != @current_color && (piece.pawn? || piece.king?)
        edge_coords = repeated_move(king_coords, move).last
        piece = edge_coords.nil? ? nil : @board.at(edge_coords)
        return true if piece && piece.beats_diagonally?
      end
      [[1, 0], [-1, 0], [0, 1], [0, -1]].each do |move|
        next_coords = relative_coords(king_coords, move)
        piece = @board.at(next_coords)
        return true if piece && piece.king?
        edge_coords = repeated_move(king_coords, move).last
        piece = edge_coords.nil? ? nil : @board.at(edge_coords)
        return true if !piece.nil? && piece.beats_straight?
      end
      [[1, 2], [2, 1], [1, -2], [-2, 1],
      [-1, 2], [2, -1], [-1, -2], [-2, -1]].each do |move|
        coords = relative_coords(king_coords, move)
        piece = possible_move?(coords) ? @board.at(coords) : nil
        return true if !piece.nil? && piece.knight?
      end
      false
    end

    ##
    # Converts a string in algebraic notation to array of coordinates
    # === Example
    #   Game.string_to_coords("a2a4") #=> [[0, 1], [0, 3]]

    def Game.string_to_coords(string)
      string = string.gsub(/\s+/, "").downcase
      raise InvalidMove, "Input must look like \"e2 e4\" or \"a6b5\"" unless /^[a-h][1-8][a-h][1-8]$/.match?(string)
      letters = ("a".."h").to_a
      [[letters.find_index(string[0]), string[1].to_i - 1],
       [letters.find_index(string[2]), string[3].to_i - 1]]
    end

    def opposite_color
      @current_color == :white ? :black : :white
    end

    def next_player
      @current_color = opposite_color
    end
  end
end

MoveValidator

module ChessEngine
  ##
  # This module contains all the methods needed to check if
  # some move is valid or not. It is included in the Game class and so uses
  # some of its attributes: board, current_color and last_piece (for en passant only)

  module MoveValidator

    ## Excludes from valid_moves all fatal moves

    def safe_moves(from)
      valid_moves(from).reject { |move| fatal_move?(from, move) }
    end

    ##
    # Returns an array of valid moves for a piece at the given position.
    # Note: this method doesn't exclude moves that lead current king to be attacked
    # (See +#safe_moves+ method)

    def valid_moves(from)
      piece = @board.at(from)
      if piece.king? || piece.knight?
        piece.moves.map do |move|
          to = relative_coords(from, move)
          to if possible_move?(to)
        end.compact
      elsif piece.pawn?
        pawn_valid_moves(from)
      else
        valid_moves_recursive(from)
      end
    end

    ##
    # Returns an array of coordinates that can be reached by recursively
    # applying the given +move+, starting from the +from+ coordinates

    private

    def repeated_move(from, move, valid_moves = [])
      coords = relative_coords(from, move)
      return valid_moves unless possible_move?(coords)
      return valid_moves << coords unless @board.at(coords).nil?
      repeated_move(coords, move, valid_moves << coords)
    end

    ##
    # Returns coordinates that will be reached after applying the +move+,
    # starting from the +from+ coordinates

    def relative_coords(from, move)
      [from[0] + move[0], from[1] + move[1]]
    end

    ##
    # Returns true if:
    # * The 8x8 board exists at given coordinates
    # * Board at given coordinates is empty or it contains a piece with the same
    #   color as the current_color

    def possible_move?(coords)
      if @board.exists_at?(coords)
        piece = @board.at(coords)
        return (piece.nil? || piece.color != @current_color)
      end
      return false
    end

    ##
    # Returns true if the current king is attacked after the given move

    def fatal_move?(from, to)
      is_fatal = false
      move = Move.new(@board, from, to)
      move.commit
      is_fatal = true if king_attacked?
      move.rollback
      is_fatal
    end

    def pawn_valid_moves(from)
      pawn = @board.at(from)
      direction = pawn.direction
      moves = []
      next_coords = relative_coords(from, [0, direction])
      jump_coords = relative_coords(from, [0, direction * 2])
      take_coords = [relative_coords(from, [1, direction]),
        relative_coords(from, [-1, direction])]
      if @board.exists_at?(next_coords) && @board.at(next_coords).nil?
        moves << next_coords
        moves << jump_coords unless pawn.moves_count > 0 || @board.at(jump_coords)
      end
      take_coords.each do |coords|
        moves << coords if @board.at(coords) && @board.at(coords).color != pawn.color
      end
      en_passant_coords(from) ? moves << en_passant_coords(from) : moves
    end

    ##
    # Returns additional valid coordinates for the pawn if available

    def en_passant_coords(from)
      pawn = @board.at(from)
      [1, -1].each do |x|
        next_coords = [from[0] + x, from[1]]
        next_piece = @board.at(next_coords)
        if next_piece.class == Pawn && next_piece == @last_piece &&
          next_piece.moves_count == 1 && from[1].between?(3, 4)
            return [from[0] + x, from[1] + pawn.direction]
        end
      end
      nil
    end

    ##
    # This method is used by #valid_moves for pieces like Queen, Rook and Elephant,
    # that should move recursively

    def valid_moves_recursive(from)
      piece = @board.at(from)
      piece.moves.inject([]) do |valid_moves, move|
        valid_moves.push(*repeated_move(from, move))
      end
    end
  end
end

Move

module ChessEngine
  ##
  # This class is made to make move canceling easier if something goes wrong.

  class Move
    def initialize(board, from, to)
      @board = board
      @from = from
      @to = to
      @original_squares = []
      @original_squares << {coords: from, piece: board.at(from)}
      @original_squares << {coords: to, piece: board.at(to)}
      if en_passant?
        @en_passant_coords = [to[0], from[1]]
        @original_squares << {coords: @en_passant_coords, piece: board.at(@en_passant_coords)}
      end
    end

    ##
    # Applies the move to the board

    def commit
      if en_passant?
        @board.set_at(@en_passant_coords, nil)
      end
      @board.move_piece(@from, @to)
    end

    ##
    # Moves pieces back and returns the board to the previous state

    def rollback
      @original_squares.each do |square|
        @board.set_at(square[:coords], square[:piece])
      end
    end

    private

    def en_passant?
      @board.at(@from).pawn? && @from[0] != @to[0] && @board.at(@to).nil?
    end
  end
end
```
\$\endgroup\$
2
  • \$\begingroup\$ This is a lot of code. You'll have better luck if you choose one of the classes you're most unsure about, and post just that for feedback. You can link to the full project for context. \$\endgroup\$
    – Jonah
    Commented Jun 3, 2019 at 3:27
  • \$\begingroup\$ @Jonah Thank you, I changed the post to make it more concise \$\endgroup\$ Commented Jun 14, 2019 at 6:28

1 Answer 1

2
\$\begingroup\$

I like move().

At least until encountering this expression at the end: ... && [7, 0].include?(to[1])

At a minimum, please define a .rank (or .row) accessor, so we can banish that magic number of 1.

Consider adding a tiny predicate that lets us write ... if piece.pawn? && piece.needs_promotion?.

I confess I don't understand the whole "promotion state" design decision. Why not just make it an optional part of a move? So we might see any of:

  • a6 a7
  • a7 a8 N
  • a7 a8 Q

Given that there's a state, I am slightly surprised we don't have "next player" taking on values of

  • white
  • white promoting a pawn
  • black
  • black promoting a pawn

but whatever. Six versus half dozen. I will keep reading to see how this pans out.


The def [](str) parsing is nice enough, I guess. It feels like turning e.g. "e4" into (4, 3) should be the Single Responsibility of some helper.

I am not especially pleased that there's two ways to return nil: valid deref or invalid coord. If caller passes in "e9", I feel a fatal error should be raised. DbC describes that as caller violating a precondition.


In promotion, kudos on assigning @promotion_coords = nil to prevent potential hard-to-diagnose bugs.

Getting the "capitalize" reference seemed like a surprisingly verbose operation, but ok, perhaps that's the best we can do.


The castling method is lovely. It's very clear.

XXX

But it doesn't appear to implement standard chess rules.

A player may not castle out of, through, or into check.


I don't write ruby code daily, so I found the !! double bang in needs_promotion? correct but a bit jargony, and hard-to-google.

Is there maybe a bool( ... ) call that would more clearly say the same thing?


In king_attacked?, the vectors of orthogonal and diagonal moves make perfect sense. But I wonder if we should rely on some common definition that other pieces can use, as well?

XXX

I don't understand this expression at all: ... && (piece.pawn? || piece.king?). Suppose White moved "c2 c4", then carnage cleared out the board, then Black's king wandered over to say hello to the other king. If Black's king manages to make a move like "d2 d3", or a move to b3, that is rare but perfectly legit.

The bug seems to stem from the fact that piece.beats_diagonally? goes "in both directions", forward and back, while pawn attacks are strictly forward. But the logic I'm reading doesn't match chess rules.

This line is correct:

        return true if piece && piece.king?

In the interest of parallel construction, I wouldn't mind seeing a piece.color guard thrown in there. Yes, yes, I know there's two kings so the guard will always evaluate true. Engineers look for such parallelism when maintaining chunks of code that have similar function.

In the final .beats_straight and !piece.nil? && piece.knight? expressions, consider sticking with the simpler idiom of piece && ... for the 1st conjunct.


I like Game.string_to_coords. I feel that def [](str) which we saw above should be calling it, so validation happens in just one place.


Here's a trivial refactor. In fatal_move?, a pair of assignments could be reduced to just

      is_fatal = king_attacked?

In en_passant_coords I found the .between?(3, 4) expression slightly surprising. It seems to conflate both en passant and normal captures. I would have expected "normal" to be handled elsewhere.

XXX

More seriously, I am concerned that we tested this for White's side of the board, but not Black's.


This code mostly implements standard chess rules.

In board.rb I would have preferred the spelling valid? over exists_at?. Each time I read "exists" it made think about a piece existing at that location, rather than about whether the location even exists. I wonder if an iteration helper could have buried the issue -- give me some proposed locations (or deltas) and I will iterate over just the ones that are valid.

Recursing on "long range" moves, rather than iterating, seemed slightly tricky.

In some places the [0] / [1] dereference felt completely natural. In others, I would have been happier to see chess .rank / .file terminology.

Overall, looks good.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.