Улучшение игры Манкала

https://www.mastersofgames.com/rules/mancala-rules.htm

Я пытаюсь создать рабочую игру Mancala в scala, в настоящее время у меня есть 3 класса, Board, Mancala и Player.

Вот копия моего Board учебный класс:

/**
 * The board of an ongoing game of mancala
 * @param player1: The number of seeds in each of lower player's pods (left to right)
 * @param bank1: The number of seeds in the lower player's bank
 * @param player2: The number of seeds in each of upper player's pods (left to right, from that player's perspective)
 * @param bank2: The number of seeds in the upper player's bank
 * @return a new board after the move takes place
 */
class Board(val player1: Seq[Int], val bank1: Int, val player2: Seq[Int], val bank2: Int) {

  /** Create a new board from a player taking a turn */
  def move(player: Int, position: Int): Board = {
    if (player != 1 && player != 2) throw new IllegalArgumentException
    if (position < 0 || position > 5) throw new IllegalArgumentException

    // Inner function for depositing seeds along a side
    def progress(seeds: Int, position: Int, side: Seq[Int]) = {
      val dist = math.min(seeds, 6 - position)
      val newSide = side.zipWithIndex.map {
        case (a, b) => if (b >= position && b < position + dist) a + 1 else a
      }
      (seeds - dist, newSide)
    }
    // Inner function for depositing a seed in the bank
    def depositInBank(seeds: Int, bank: Int): (Int, Int) = {
      if (seeds > 0) (seeds - 1, bank + 1) else (seeds, bank)
    }

    // Redistribute the pieces. Board movement positions are always counted
    // from the left
    val (startSide, oppSide) =
    if (player == 1) (player1.toArray, player2.reverse.toArray)
    else (player2.reverse.toArray, player1.toArray)
    val start = if (player == 1) position else 5 - position
    val playerBank = if (player == 1) bank1 else bank2
    if (startSide(start) == 0) throw new IllegalStateException
    val seeds = startSide(start)
    startSide(start) = 0
    val pos = start + 1

    // First side pass
    val (seedsFirstPass, playerPodsFirstPass) = progress(seeds, pos, startSide)
    val (seedsFirstPassAndBank, playerBankFirstPass) = depositInBank(seedsFirstPass, playerBank)

    // Move around the other side of the board
    val (seedsSecondPass, oppSideSecondPass) = progress(seedsFirstPassAndBank, 0, oppSide)

    // Skip the opponent's bank, but keep going if we can
    val (seedsThirdPass, playerPodsThirdPass) = progress(seedsSecondPass, 0, playerPodsFirstPass)

    // Stop by the bank
    val (seedsThirdsPassAndBank, playerBankSecondPass) = depositInBank(seedsThirdPass, playerBankFirstPass)

    // Just to be safe
    val (seedsFourthPass, oppSideFourthPass) = progress(seedsThirdsPassAndBank, 0, oppSideSecondPass)

    // Make our assumption explicit
    assert(seedsFourthPass == 0, "Still seeds after fourth pass, did not account for that")

    // Construct the new board
    if (player == 1)
      new Board(playerPodsThirdPass.toVector, playerBankSecondPass, oppSideFourthPass.reverse.toVector, bank2)
    else
      new Board(oppSideFourthPass.toVector, bank1, playerPodsThirdPass.reverse.toVector, playerBankSecondPass)
  }

  /** Capture pieces from a mancala game
   *
   * Pieces from the capturing pod and the opposite pod (same position, other player) are all emptied and moved
   * to the capturing player's bank.
   *
   * @param player the side (1 or 2) doing the capturing
   * @param pos the position doing the capturing
   * @return a new mancala board after the capture
   */
  def capture(player: Int, pos: Int): Board = {
    assert(player1(pos) != 0 && player2(pos) != 0)
    val (newBank1, newBank2) =
      if (player == 1)
        (bank1 + player1(pos) + player2(pos), bank2)
      else
        (bank1, bank2 + player1(pos) + player2(pos))
    new Board(player1.updated(pos, 0), newBank1, player2.updated(pos, 0), newBank2)
  }

  /** Calculate the last pod that will be visited by a move
   *
   * @param player the number (1, 2) of the player making the move
   * @param pos the index of the move about to be made (always counting from the left)
   * @return a tuple (player, pos) that represents the players side and position the last seed will be deposited in.
   *         A position of 6 represents a player's bank.
   */
  def lastPosAfterMove(player: Int, pos: Int): (Int, Int) = {
    val seeds = (if (player == 1) player1 else player2)(pos)
    val distToBank = if (player == 1) 6 - pos else pos + 1

    // Could use a better approach
    if (player == 1)
      if (seeds < distToBank) (1, pos + seeds)
      else if (seeds == distToBank) (1, 6)
      else if (seeds <= distToBank + 6) (2, 6 - (seeds - distToBank))
      else if (seeds <= distToBank + 12) (1, seeds - distToBank - 7)
      else if (seeds == distToBank + 13) (1, 6)
      else if (seeds <= distToBank + 19) (2, 19 - (seeds - distToBank))
      else { assert(false); (0, 0) }
    else
      if (seeds < distToBank) (2, pos - seeds)
      else if (seeds == distToBank) (2, 6)
      else if (seeds <= distToBank + 6) (1, seeds - distToBank - 1)
      else if (seeds <= distToBank + 12) (2, 12 - (seeds - distToBank))
      else if (seeds == distToBank + 13) (2, 6)
      else if (seeds <= distToBank + 19) (1, seeds - distToBank - 14)
      else { assert(false); (0, 0) }
  }

  override def toString = {
    val p2 = player2.foldLeft("")((a, b) => a + StringContext("  ", "  ").f(b)).trim
    val p1 = player1.foldLeft("")((a, b) => a + StringContext("  ", "  ").f(b)).trim
    val layer = "-" * 32
    val space = " " * 28
    f"""${layer + " "}n   $p2 n$bank2 ${space} $bank1n   $p1n${layer}"""
  }
}

/**
 * Board companion object. Used as a factory to create a starting board easily
 */
object Board {
  /** Create an initial board */
  def apply() = {
    val boardSide = Seq(4,4,4,4,4,4)
    new Board(boardSide, 0, boardSide, 0)
  }
}

Я пытаюсь улучшить lastPosAfterMove() и move() функций, чтобы сделать их более компактными и понятными для других и использовать лучший подход.

Пока не видел других классов, этот пока выглядит неплохо.

• Комментарии полезные. Это чудесно!
• val space = " " * 28 прекрасный.
• Именование переменных хорошее и последовательное. Я обычно выступаю за прописанные имена переменных (positionи т. д.), но здесь все не так уж и плохо.

Однако я бы подумал о нескольких вещах:

• move очень долго. Вы уже создали вспомогательные функции, поэтому я бы переместил их и сделал частными.
• В IllegalArgumentExceptions может использовать описательное сообщение, например
  throw new IllegalArgumentException(s"invalid player value $player"). А еще лучше избегать ситуации путем построения (см. Ниже).
• Отступ между val (startSide, oppSide) = и
  val pos = start + 1 странно, я бы дал ему еще несколько пустых строк, чтобы было более очевидно, что происходит. Также переместите проверки ближе к тому месту, где установлены переменные (if (startSide(start) == 0 ... вероятно должен прийти после
  val start =), чтобы соединять связанные вещи.
• Рассмотрите возможность именования аргументов, хотя некоторые IDE все равно их покажут.
• Что именно означает «на всякий случай»?
• Все toVector и toArray звонки необходимы? Я бы подумал, что нет конверсии для Seqs здесь вообще должно быть необходимо? Может быть другого типа, чем Seq следует использовать здесь, если в любом случае требуется произвольный доступ?
• Если есть только два игрока, подумайте о том, чтобы не делать это неявным с использованием 1 и 2, а использовать перечисление, чтобы вы не могу попасть в состояние, когда player переменная может быть чем-то неожиданным. То же самое на самом деле касается position, подумайте о том, чтобы сделать его классом case, чтобы четко различать специальные значения позиций (например, 6 здесь).
• «Можно было бы использовать лучший подход», это правда, это немного повторяется, но с другой стороны, ясно, что происходит, может, на самом деле ничего не делать с этим. В assert(false); {0, 0) должен быть
  IllegalStateException или что-то в этом роде, если вы не можете исключить незаконное состояние другими способами (assert(false)
  ничего не говорит читателю). Продолжение с недопустимым состоянием в целом очень плохо, и здесь можно легко использовать исключения, чтобы этого избежать.
• toString можно опустить {} части для простой переменной.
• Моя IDE также предлагает дать каждому публичному члену объявление типа, я склонен с этим согласиться. В этом случае progress также должен иметь объявленный тип, это помогает читать все это целиком.
• В коде есть несколько магических констант, конечно 0, хотя я бы сказал, что это не в счет, но 6 определенно учитываются. Подумайте о том, чтобы сделать это постоянным и использовать его в соответствующих местах, даже для 5 (Board.SIZE - 1 или что-то в этом роде). Да, это никогда не изменится, но это проясняет намерение читателя. Я недостаточно понимаю логику, чтобы применить это предложение к lastPosAfterMove, но это применимо и к 7, 12, 13, 19 и т. д., предположительно, они определяются размером платы.
• Я бы встроил pos.
• Не совсем уверен, что без запуска foldLeft последовательность работает, но именно по этой причине я бы сделал ее вспомогательной функцией и повторно использовал ее один раз.

Некоторые стилистические идеи, в которых я абсолютно уверен:

• Можно использовать диапазоны, например !(0 until 6).contains(position) для проверки действительности. Мне кажется понятнее, но YMMV.

Надеюсь, я ничего не перепутал, при этом я бы, вероятно, поступил так, как описано выше:

/**
 * The board of an ongoing game of mancala
 *
 * @param player1 : The number of seeds in each of lower player's pods (left to right)
 * @param bank1   : The number of seeds in the lower player's bank
 * @param player2 : The number of seeds in each of upper player's pods (left to right, from that player's perspective)
 * @param bank2   : The number of seeds in the upper player's bank
 * @return a new board after the move takes place
 */
class Board(val player1: IndexedSeq[Int], val bank1: Int, val player2: IndexedSeq[Int], val bank2: Int) {

  import Board._

  object Player extends Enumeration {
    type Player = Value
    val First, Second = Value
  }

  import Player._

  // Depositing seeds along a side
  private def progress(seeds: Int, position: Int, side: IndexedSeq[Int]): (Int, IndexedSeq[Int]) = {
    val dist = math.min(seeds, Size - position)
    val newSide = side.zipWithIndex.map {
      case (a, b) => if ((position until position + dist).contains(b)) a + 1 else a
    }
    (seeds - dist, newSide)
  }

  private def depositInBank(seeds: Int, bank: Int): (Int, Int) =
    if (seeds > 0) (seeds - 1, bank + 1) else (seeds, bank)

  /** Create a new board from a player taking a turn */
  def move(player: Player, position: Int): Board = {
    if (!(0 until Size).contains(position)) throw new IllegalArgumentException(s"invalid position value $position is out of bounds")

    // Redistribute the pieces. Board movement positions are always counted
    // from the left
    val (startSide, oppSide) =
    if (player == First) (player1.toArray, player2.reverse.toArray)
    else (player2.reverse.toArray, player1.toArray)

    val start = if (player == First) position else Size - 1 - position
    if (startSide(start) == 0) throw new IllegalStateException(s"invalid value for $start ...")

    val playerBank = if (player == First) bank1 else bank2

    val seeds = startSide(start)
    startSide(start) = 0

    // First side pass
    val (seedsFirstPass, playerPodsFirstPass) = progress(seeds, position = start + 1, startSide)
    val (seedsFirstPassAndBank, playerBankFirstPass) = depositInBank(seedsFirstPass, playerBank)

    // Move around the other side of the board
    val (seedsSecondPass, oppSideSecondPass) = progress(seedsFirstPassAndBank, position = 0, oppSide)

    // Skip the opponent's bank, but keep going if we can
    val (seedsThirdPass, playerPodsThirdPass) = progress(seedsSecondPass, position = 0, playerPodsFirstPass)

    // Stop by the bank
    val (seedsThirdsPassAndBank, playerBankSecondPass) = depositInBank(seedsThirdPass, playerBankFirstPass)

    // Just to be safe
    val (seedsFourthPass, oppSideFourthPass) = progress(seedsThirdsPassAndBank, position = 0, oppSideSecondPass)

    // Make our assumption explicit
    assert(seedsFourthPass == 0, "Still seeds after fourth pass, did not account for that")

    // Construct the new board
    if (player == First)
      new Board(playerPodsThirdPass, playerBankSecondPass, oppSideFourthPass.reverse, bank2)
    else
      new Board(oppSideFourthPass, bank1, playerPodsThirdPass.reverse, playerBankSecondPass)
  }

  /** Capture pieces from a mancala game
   *
   * Pieces from the capturing pod and the opposite pod (same position, other player) are all emptied and moved
   * to the capturing player's bank.
   *
   * @param player the side doing the capturing
   * @param pos    the position doing the capturing
   * @return a new mancala board after the capture
   */
  def capture(player: Player, pos: Int): Board = {
    assert(player1(pos) != 0 && player2(pos) != 0)
    val toAdd = player1(pos) + player2(pos)
    val (newBank1, newBank2) =
      if (player == First)
        (bank1 + toAdd, bank2)
      else
        (bank1, bank2 + toAdd)
    new Board(player1.updated(pos, 0), newBank1, player2.updated(pos, 0), newBank2)
  }

  /** Calculate the last pod that will be visited by a move
   *
   * @param player the player making the move
   * @param pos    the index of the move about to be made (always counting from the left)
   * @return a tuple (player, pos) that represents the player and position the last seed will be deposited in.
   *         A position of 6 represents a player's bank.
   */
  def lastPosAfterMove(player: Player, pos: Int): (Player, Int) = {
    val seeds = (if (player == First) player1 else player2) (pos)
    val distToBank = if (player == First) Size - pos else pos + 1

    if (player == First)
      if (seeds < distToBank) (First, pos + seeds)
      else if (seeds == distToBank) (First, Size)
      else if (seeds <= distToBank + Size) (Second, Size - (seeds - distToBank))
      else if (seeds <= distToBank + 12) (First, seeds - distToBank - 7)
      else if (seeds == distToBank + 13) (First, Size)
      else if (seeds <= distToBank + 19) (Second, 19 - (seeds - distToBank))
      else throw new IllegalStateException(s"value for seeds $seeds out of bounds")
    else
      if (seeds < distToBank) (Second, pos - seeds)
      else if (seeds == distToBank) (Second, Size)
      else if (seeds <= distToBank + Size) (First, seeds - distToBank - 1)
      else if (seeds <= distToBank + 12) (Second, 12 - (seeds - distToBank))
      else if (seeds == distToBank + 13) (Second, Size)
      else if (seeds <= distToBank + 19) (First, seeds - distToBank - 14)
      else throw new IllegalStateException(s"value for seeds $seeds out of bounds")
  }

  private def foo(x: IndexedSeq[Int]): String =
    x.foldLeft("")((a, b) => a + StringContext("  ", "  ").f(b)).trim

  override def toString: String = {
    val layer = "-" * 32
    val space = " " * 30
    f"""$layern   ${foo(player2)}n$bank2$space$bank1n   ${foo(player1)}n$layer"""
  }
}

/**
 * Board companion object. Used as a factory to create a starting board easily
 */
object Board {
  val Size = 6

  /** Create an initial board */
  def apply(): Board = {
    val boardSide = List.fill(Size)(4).toVector
    new Board(boardSide, bank1 = 0, boardSide, bank2 = 0)
  }
}