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

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()` функций, чтобы сделать их более компактными и понятными для других и использовать лучший подход.

## 1 ответ

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

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

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

• `move` очень долго. Вы уже создали вспомогательные функции, поэтому я бы переместил их и сделал частными.
• В `IllegalArgumentException`s может использовать описательное сообщение, например
`throw new IllegalArgumentException(s"invalid player value \$player")`. А еще лучше избегать ситуации путем построения (см. Ниже).
• Отступ между `val (startSide, oppSide) =` и
`val pos = start + 1` странно, я бы дал ему еще несколько пустых строк, чтобы было более очевидно, что происходит. Также переместите проверки ближе к тому месту, где установлены переменные (`if (startSide(start) == 0 ...` вероятно должен прийти после
`val start =`), чтобы соединять связанные вещи.
• Рассмотрите возможность именования аргументов, хотя некоторые IDE все равно их покажут.
• Что именно означает «на всякий случай»?
• Все `toVector` и `toArray` звонки необходимы? Я бы подумал, что нет конверсии для `Seq`s здесь вообще должно быть необходимо? Может быть другого типа, чем `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)
else
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)
}
}
``````