3
\$\begingroup\$

This is an updated version of my first question: Chess game in Kotlin

I wrote the following Chess logic in Kotlin and am looking for feedback to make the code cleaner and follow good software design principles. I tried to adhere to object-oriented design.

Some notes and questions I still have:

• I ignored special moves like "Castling" and "en pessant" for simplicity.

• In getAvailableMoves I put each branch into its own if-statement because a Queen gets its moves from two branches (the if for diagonal moves which also handles the Bishop and the if for orthogonal moves together with the Rook). I could put the other branches into else-ifs but I found it more readable if it's consistent. What do you think? Does the performance cost matter?

• I didn't really know how to get rid of duplication in some of the getAvailableMoves branches, like the Rook one because the increment operator for each direction is different (toXUp++, toXDown++, toYLeft--, toYRight++..)

• Is it okay to mutate the Pawn's hasStartingPosition value or should I make it a val call copy instead?

• Is it good to make both the Piece and the Player property of a PlayerPiece nullable? My first attempt included a Player.None and Piece.Empty class but then I always have to handle these cases in when expressions.

• To reset the game you initialize a new ChessGame object. This design decision was done because of the UI framework I'm using (Jetpack Compose). Would it be better to add a reset method like this instead?

fun resetGame() {
    positionsArray = getStartingPositions()
    currentPlayer = Player.White
    removedPiecesList = mutableListOf()
}

The code itself:

class ChessGame {

    sealed class Piece {
        object King : Piece()
        object Queen : Piece()
        object Bishop : Piece()
        object Knight : Piece()
        object Rook : Piece()
        data class Pawn(var hasStartingPosition: Boolean = true) : Piece()
    }

    enum class Player {
        Black, White
    }

    data class PlayerPiece(val piece: Piece? = null, val player: Player? = null)

    var currentPlayer: Player = Player.White

    private var playingFieldArray = getStartingPositions()
    val playingField: Array<Array<PlayerPiece>> = playingFieldArray

    private fun getStartingPositions() = arrayOf(
        arrayOf(
            PlayerPiece(Piece.Rook, Player.Black),
            PlayerPiece(Piece.Knight, Player.Black),
            PlayerPiece(Piece.Bishop, Player.Black),
            PlayerPiece(Piece.Queen, Player.Black),
            PlayerPiece(Piece.King, Player.Black),
            PlayerPiece(Piece.Bishop, Player.Black),
            PlayerPiece(Piece.Knight, Player.Black),
            PlayerPiece(Piece.Rook, Player.Black),
        ),
        arrayOf(
            PlayerPiece(Piece.Pawn(), Player.Black),
            PlayerPiece(Piece.Pawn(), Player.Black),
            PlayerPiece(Piece.Pawn(), Player.Black),
            PlayerPiece(Piece.Pawn(), Player.Black),
            PlayerPiece(Piece.Pawn(), Player.Black),
            PlayerPiece(Piece.Pawn(), Player.Black),
            PlayerPiece(Piece.Pawn(), Player.Black),
            PlayerPiece(Piece.Pawn(), Player.Black)
        ),
        Array(8) { PlayerPiece() },
        Array(8) { PlayerPiece() },
        Array(8) { PlayerPiece() },
        Array(8) { PlayerPiece() },
        arrayOf(
            PlayerPiece(Piece.Pawn(), Player.White),
            PlayerPiece(Piece.Pawn(), Player.White),
            PlayerPiece(Piece.Pawn(), Player.White),
            PlayerPiece(Piece.Pawn(), Player.White),
            PlayerPiece(Piece.Pawn(), Player.White),
            PlayerPiece(Piece.Pawn(), Player.White),
            PlayerPiece(Piece.Pawn(), Player.White),
            PlayerPiece(Piece.Pawn(), Player.White)
        ),
        arrayOf(
            PlayerPiece(Piece.Rook, Player.White),
            PlayerPiece(Piece.Knight, Player.White),
            PlayerPiece(Piece.Bishop, Player.White),
            PlayerPiece(Piece.Queen, Player.White),
            PlayerPiece(Piece.King, Player.White),
            PlayerPiece(Piece.Bishop, Player.White),
            PlayerPiece(Piece.Knight, Player.White),
            PlayerPiece(Piece.Rook, Player.White)
        ),
    )

    private var removedPiecesList = mutableListOf<PlayerPiece>()
    val removedPieces: List<PlayerPiece> = removedPiecesList

    fun getAvailableMoves(x: Int, y: Int): List<Point> {
        val field = playingFieldArray[x][y]
        if (field.player != currentPlayer || isGameOver()) {
            return emptyList()
        }

        val availableMoves = mutableListOf<Point>()

        fun isValidPosition(x: Int, y: Int) = x in 0..7 && y in 0..7 && !tileHasPieceOfCurrentPlayer(x, y)

        if (field.piece == Piece.Rook || field.piece == Piece.Queen) {
            var toXUp = x - 1
            val toYUp = y
            while (isValidPosition(toXUp, toYUp)
                && !tileHasPieceOfCurrentPlayer(toXUp, toYUp)
            ) {
                availableMoves.add(Point(toXUp, toYUp))
                if (tileHasPieceOfOpponent(toXUp, toYUp)) break
                toXUp--
            }
            var toXDown = x + 1
            val toYDown = y
            while (isValidPosition(toXDown, toYDown)
                && !tileHasPieceOfCurrentPlayer(toXDown, toYDown)
            ) {
                availableMoves.add(Point(toXDown, toYDown))
                if (tileHasPieceOfOpponent(toXDown, toYDown)) break
                toXDown++
            }
            val toXLeft = x
            var toYLeft = y - 1
            while (isValidPosition(toXLeft, toYLeft)
                && !tileHasPieceOfCurrentPlayer(toXLeft, toYLeft)
            ) {
                availableMoves.add(Point(toXLeft, toYLeft))
                if (tileHasPieceOfOpponent(toXLeft, toYLeft)) break
                toYLeft--
            }
            val toXRight = x
            var toYRight = y + 1
            while (isValidPosition(toXRight, toYRight)
                && !tileHasPieceOfCurrentPlayer(toXRight, toYRight)
            ) {
                availableMoves.add(Point(toXRight, toYRight))
                if (tileHasPieceOfOpponent(toXRight, toYRight)) break
                toYRight++
            }
        }

        if (field.piece == Piece.Knight) {
            listOf(
                Point(x - 2, y - 1), Point(x - 2, y + 1), Point(x + 2, y - 1), Point(x + 2, y + 1),
                Point(x - 1, y - 2), Point(x - 1, y + 2), Point(x + 1, y - 2), Point(x + 1, y + 2)
            ).forEach { point ->
                if (isValidPosition(point.x, point.y)) {
                    availableMoves.add(point)
                }
            }
        }

        if (field.piece == Piece.King) {
            listOf(
                Point(x - 1, y), Point(x + 1, y), Point(x, y - 1), Point(x, y + 1), Point(x - 1, y - 1),
                Point(x - 1, y + 1), Point(x + 1, y - 1), Point(x + 1, y + 1)
            ).forEach { point ->
                if (isValidPosition(point.x, point.y)) {
                    availableMoves.add(point)
                }
            }
        }

        if (field.piece is Piece.Pawn) {
            if (field.player == Player.Black) {
                val toXDown = x + 1
                val toYDown = y
                if (isValidPosition(toXDown, toYDown) && !tileHasPieceOfOpponent(toXDown, toYDown)) {
                    availableMoves.add(Point(toXDown, toYDown))
                }

                if (field.piece.hasStartingPosition) {
                    val toXDown2 = x + 2
                    val toYDown2 = y
                    if (isValidPosition(toXDown2, toYDown2) && !tileHasPieceOfOpponent(toXDown2, toYDown2)) {
                        availableMoves.add(Point(toXDown2, toYDown2))
                    }
                }

                listOf(
                    Point(x + 1, y + 1), Point(x + 1, y - 1)
                ).forEach { point ->
                    if (isValidPosition(point.x, point.y)
                        && tileHasPieceOfOpponent(point.x, point.y)
                    ) {
                        availableMoves.add(point)
                    }
                }
            } else {
                val toXUp = x - 1
                val toYUp = y
                if (isValidPosition(toXUp, toYUp) && !tileHasPieceOfOpponent(toXUp, toYUp)) {
                    availableMoves.add(Point(toXUp, toYUp))
                }

                if (field.piece.hasStartingPosition) {
                    val toXDown2 = x - 2
                    val toYDown2 = y
                    if (isValidPosition(toXDown2, toYDown2) && !tileHasPieceOfOpponent(toXDown2, toYDown2)) {
                        availableMoves.add(Point(toXDown2, toYDown2))
                    }
                }

                listOf(
                    Point(x - 1, y + 1), Point(x - 1, y - 1)
                ).forEach { point ->
                    if (isValidPosition(point.x, point.y)
                        && tileHasPieceOfOpponent(point.x, point.y)
                    ) {
                        availableMoves.add(point)
                    }
                }
            }
        }

        if (field.piece == Piece.Bishop || field.piece == Piece.Queen) {
            var toXUpLeft = x - 1
            var toYUpLeft = y - 1
            while (isValidPosition(toXUpLeft, toYUpLeft)
                && !tileHasPieceOfCurrentPlayer(toXUpLeft, toYUpLeft)
            ) {
                availableMoves.add(Point(toXUpLeft, toYUpLeft))
                if (tileHasPieceOfOpponent(toXUpLeft, toYUpLeft)) break
                toXUpLeft--
                toYUpLeft--
            }
            var toXUpRight = x - 1
            var toYUpRight = y + 1
            while (isValidPosition(toXUpRight, toYUpRight)
                && !tileHasPieceOfCurrentPlayer(toXUpRight, toYUpRight)
            ) {
                availableMoves.add(Point(toXUpRight, toYUpRight))
                if (tileHasPieceOfOpponent(toXUpRight, toYUpRight)) break
                toXUpRight--
                toYUpRight++
            }
            var toXDownLeft = x + 1
            var toYDownLeft = y - 1
            while (isValidPosition(toXDownLeft, toYDownLeft)
                && !tileHasPieceOfCurrentPlayer(toXDownLeft, toYDownLeft)
            ) {
                availableMoves.add(Point(toXDownLeft, toYDownLeft))
                if (tileHasPieceOfOpponent(toXDownLeft, toYDownLeft)) break
                toXDownLeft++
                toYDownLeft--
            }
            var toXDownRight = x + 1
            var toYDownRight = y + 1
            while (isValidPosition(toXDownRight, toYDownRight)
                && !tileHasPieceOfCurrentPlayer(toXDownRight, toYDownRight)
            ) {
                availableMoves.add(Point(toXDownRight, toYDownRight))
                if (tileHasPieceOfOpponent(toXDownRight, toYDownRight)) break
                toXDownRight++
                toYDownRight++
            }
        }

        return availableMoves
    }

    fun movePiece(fromX: Int, fromY: Int, toX: Int, toY: Int) {
        if (getAvailableMoves(fromX, fromY).contains(Point(toX, toY))) {
            if (tileHasPieceOfOpponent(toX, toY)) {
                removedPiecesList.add(playingField[toX][toY])
            }
            playingFieldArray[toX][toY] = playingFieldArray[fromX][fromY]
            playingFieldArray[fromX][fromY] = PlayerPiece()
            (playingFieldArray[toX][toY].piece as? Piece.Pawn)?.hasStartingPosition = false
        } else {
            throw IllegalArgumentException("Invalid move coordinates")
        }
        currentPlayer = if (currentPlayer == Player.White) Player.Black else Player.White
    }

    fun tileHasPieceOfCurrentPlayer(x: Int, y: Int) = when (currentPlayer) {
        Player.Black -> {
            playingField[x][y].player == Player.Black
        }
        Player.White -> {
            playingField[x][y].player == Player.White
        }
    }

    private fun tileHasPieceOfOpponent(x: Int, y: Int) = when (currentPlayer) {
        Player.Black -> {
            playingField[x][y].player == Player.White
        }
        Player.White -> {
            playingField[x][y].player == Player.Black
        }
    }

    fun isGameOver() = removedPieces.any { it.piece == Piece.King }
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Your implementation is very much procedural as can be seen in getAvailableMoves. Since you are interested in OOO, you may want to take a look at the "Anemic domain model" anti-pattern described here: en.wikipedia.org/wiki/Anemic_domain_model \$\endgroup\$ Apr 24, 2021 at 11:27

2 Answers 2

2
\$\begingroup\$

I'd start with defining simple, self-contained things, such as a very generic vector:

data class Vector(val x: Int, val y: Int) {
    companion object {
        val perpendicular = listOf(Vector(-1, 0), Vector(+1, 0), Vector(0, -1), Vector(0, +1))
        val diagonal = listOf(Vector(-1, -1), Vector(+1, +1), Vector(-1, +1), Vector(+1, -1))
        val lShaped = ...
    }
}

and a point that I can add vectors to

data class Point(val x: Int, val y: Int) {
    operator fun plus(vector: Vector) = Point(x + vector.x, y + vector.y)

    fun isValid() = x in 0..7 && y in 0..7
}

If we ignore castling etc, there are basically two kinds of moves, one is adding a vector once, and another is adding it several times. This can be a property of the move, or the piece. Suppose it's a property of the move:

interface Move {
    fun validPointsForStartingPoint(point: Point): List<Point>

    class Step(private val vector: Vector) : Move {
        override fun validPointsForStartingPoint(point: Point) = listOf(point + vector).filter { it.isValid() }
    }

    class Linear(private val vector: Vector) : Move {
        override fun validPointsForStartingPoint(point: Point) = sequence {
            var nextPoint = point + vector
            while (nextPoint.isValid()) {
                yield(nextPoint)
                nextPoint += vector
            }
        }.toList()
    }

    companion object {
        val linearPerpendicular = Vector.perpendicular.map { Linear(it) }
        val linearDiagonal = Vector.diagonal.map { Linear(it) }

        ...
    }
}

And moves are the properties of pieces:

sealed class Piece(val availableMoves: List<Move>) {
    object Queen : Piece(Move.linearPerpendicular + Move.linearDiagonal)
    object Bishop : Piece(Move.linearDiagonal)
    object Rook : Piece(Move.linearPerpendicular)
    ...
}

Then finding available moves becomes easy:

fun Piece.getAvailableMovePointsForPoint(point: Point) = sequence {
    availableMoves.forEach move@{ move ->
        move.validPointsForStartingPoint(point).forEach { point -> 
            if (point.occupiedByOwnPiece()) return@move
            yield(move)
            if (point.occupiedByEnemyPiece()) return@move
        }
    }
}

This is not the best approach but maybe you can get some ideas out of this

\$\endgroup\$
1
  • \$\begingroup\$ “Cardinal” and “ordinal” would be more descriptive of those lists than “perpendicular” or “diagonal”. Perpendicular to what? Both lists contain some vectors that are perpendicular to each other, and the same would be true for the moves a knight could make. \$\endgroup\$
    – Tenfour04
    Sep 11, 2023 at 21:58
0
\$\begingroup\$

This kind of logic:

private fun tileHasPieceOfOpponent(x: Int, y: Int) = when (currentPlayer) {
    Player.Black -> {
        playingField[x][y].player == Player.White
    }
    Player.White -> {
        playingField[x][y].player == Player.Black
    }
}

Could be simplified if you had a helper property in your enum class:

val opposing: Player 
    get() = if (this == Black) White else Black
private fun tileHasPieceOfOpponent(x: Int, y: Int) = 
    playingField[x][y].player == player.opposing
   
\$\endgroup\$
1
  • \$\begingroup\$ Other than that, I would focus on the breaking down the highly procedural code. The other answer would be a great first step towards that. \$\endgroup\$
    – Tenfour04
    Sep 11, 2023 at 22:12

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.