Chess game in Kotlin

Question

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:

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

• I put the last when branch into a separate when statement so that the orthogonal and diagonal Queen moves can add up.

• To reset the game you initialize a new ChessGame object. 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 {
 var currentPlayer: Player = Player.White
 private var playingFieldArray = getStartingPositions()
 val playingField: Array<Array<Piece>> = playingFieldArray
 private fun getStartingPositions() = arrayOf<Array<Piece>>(
 arrayOf(
 Piece.Black.Rook1,
 Piece.Black.Knight1,
 Piece.Black.Bishop1,
 Piece.Black.Queen,
 Piece.Black.King,
 Piece.Black.Bishop2,
 Piece.Black.Knight2,
 Piece.Black.Rook2
 ),
 arrayOf(
 Piece.Black.Pawn1,
 Piece.Black.Pawn2,
 Piece.Black.Pawn3,
 Piece.Black.Pawn4,
 Piece.Black.Pawn5,
 Piece.Black.Pawn6,
 Piece.Black.Pawn7,
 Piece.Black.Pawn8
 ),
 arrayOf(
 Piece.Empty,
 Piece.Empty,
 Piece.Empty,
 Piece.Empty,
 Piece.Empty,
 Piece.Empty,
 Piece.Empty,
 Piece.Empty
 ),
 arrayOf(
 Piece.Empty,
 Piece.Empty,
 Piece.Empty,
 Piece.Empty,
 Piece.Empty,
 Piece.Empty,
 Piece.Empty,
 Piece.Empty
 ),
 arrayOf(
 Piece.Empty,
 Piece.Empty,
 Piece.Empty,
 Piece.Empty,
 Piece.Empty,
 Piece.Empty,
 Piece.Empty,
 Piece.Empty
 ),
 arrayOf(
 Piece.Empty,
 Piece.Empty,
 Piece.Empty,
 Piece.Empty,
 Piece.Empty,
 Piece.Empty,
 Piece.Empty,
 Piece.Empty
 ),
 arrayOf(
 Piece.White.Pawn1,
 Piece.White.Pawn2,
 Piece.White.Pawn3,
 Piece.White.Pawn4,
 Piece.White.Pawn5,
 Piece.White.Pawn6,
 Piece.White.Pawn6,
 Piece.White.Pawn8
 ),
 arrayOf(
 Piece.White.Rook1,
 Piece.White.Knight1,
 Piece.White.Bishop1,
 Piece.White.Queen,
 Piece.White.King,
 Piece.White.Bishop2,
 Piece.White.Knight2,
 Piece.White.Rook2
 ),
 )
 private var removedPiecesList = mutableListOf<Piece>()
 val removedPieces: List<Piece> = removedPiecesList
 fun getAvailableMoves(x: Int, y: Int): List<Pair<Int, Int>> {
 if (playingFieldArray[x][y] is Piece.White && currentPlayer == Player.Black ||
 playingFieldArray[x][y] is Piece.Black && currentPlayer == Player.White ||
 isGameOver()
 ) {
 return emptyList()
 }
 val availableMoves = mutableListOf<Pair<Int, Int>>()
 fun isValidPosition(x: Int, y: Int) = x in 0..7 && y in 0..7 && !tileHasPieceOfCurrentPlayer(x, y)
 when (playingFieldArray[x][y]) {
 Piece.Black.Rook1, Piece.Black.Rook2, Piece.White.Rook1, Piece.White.Rook2, Piece.Black.Queen, Piece.White.Queen -> {
 var toXUp = x - 1
 val toYUp = y
 while (isValidPosition(toXUp, toYUp)
 && !tileHasPieceOfCurrentPlayer(toXUp, toYUp)
 ) {
 availableMoves.add(Pair(toXUp, toYUp))
 if (tileHasPieceOfOpponent(toXUp, toYUp)) break
 toXUp--
 }
 var toXDown = x + 1
 val toYDown = y
 while (isValidPosition(toXDown, toYDown)
 && !tileHasPieceOfCurrentPlayer(toXDown, toYDown)
 ) {
 availableMoves.add(Pair(toXDown, toYDown))
 if (tileHasPieceOfOpponent(toXDown, toYDown)) break
 toXDown++
 }
 val toXLeft = x
 var toYLeft = y - 1
 while (isValidPosition(toXLeft, toYLeft)
 && !tileHasPieceOfCurrentPlayer(toXLeft, toYLeft)
 ) {
 availableMoves.add(Pair(toXLeft, toYLeft))
 if (tileHasPieceOfOpponent(toXLeft, toYLeft)) break
 toYLeft--
 }
 val toXRight = x
 var toYRight = y + 1
 while (isValidPosition(toXRight, toYRight)
 && !tileHasPieceOfCurrentPlayer(toXRight, toYRight)
 ) {
 availableMoves.add(Pair(toXRight, toYRight))
 if (tileHasPieceOfOpponent(toXRight, toYRight)) break
 toYRight++
 }
 }
 Piece.Black.Knight1, Piece.Black.Knight2, Piece.White.Knight1, Piece.White.Knight2 -> {
 val toXUpLeft = x - 2
 val toYUpLeft = y - 1
 if (isValidPosition(toXUpLeft, toYUpLeft)) {
 availableMoves.add(Pair(toXUpLeft, toYUpLeft))
 }
 val toXUpRight = x - 2
 val toYUpRight = y + 1
 if (isValidPosition(toXUpRight, toYUpRight)) {
 availableMoves.add(Pair(toXUpRight, toYUpRight))
 }
 val toXDownLeft = x + 2
 val toYDownLeft = y - 1
 if (isValidPosition(toXDownLeft, toYDownLeft)) {
 availableMoves.add(Pair(toXDownLeft, toYDownLeft))
 }
 val toXDownRight = x + 2
 val toYDownRight = y + 1
 if (isValidPosition(toXDownRight, toYDownRight)) {
 availableMoves.add(Pair(toXDownRight, toYDownRight))
 }
 val toXLeftUp = x - 1
 val toYLeftUp = y - 2
 if (isValidPosition(toXLeftUp, toYLeftUp)) {
 availableMoves.add(Pair(toXLeftUp, toYLeftUp))
 }
 val toXRightUp = x - 1
 val toYRightUp = y + 2
 if (isValidPosition(toXRightUp, toYRightUp)) {
 availableMoves.add(Pair(toXRightUp, toYRightUp))
 }
 val toXLeftDown = x + 1
 val toYLeftDown = y - 2
 if (isValidPosition(toXLeftDown, toYLeftDown)) {
 availableMoves.add(Pair(toXLeftDown, toYLeftDown))
 }
 val toXRightDown = x + 1
 val toYRightDown = y + 2
 if (isValidPosition(toXRightDown, toYRightDown)) {
 availableMoves.add(Pair(toXRightDown, toYRightDown))
 }
 }
 Piece.Black.King, Piece.White.King -> {
 val toXUp = x - 1
 val toYUp = y
 if (isValidPosition(toXUp, toYUp)) {
 availableMoves.add(Pair(toXUp, toYUp))
 }
 val toXDown = x + 1
 val toYDown = y
 if (isValidPosition(toXDown, toYDown)) {
 availableMoves.add(Pair(toXDown, toYDown))
 }
 val toXLeft = x
 val toYLeft = y - 1
 if (isValidPosition(toXLeft, toYLeft)) {
 availableMoves.add(Pair(toXLeft, toYLeft))
 }
 val toXRight = x
 val toYRight = y + 1
 if (isValidPosition(toXRight, toYRight)) {
 availableMoves.add(Pair(toXRight, toYRight))
 }
 val toXUpLeft = x - 1
 val toYUpLeft = y - 1
 if (isValidPosition(toXUpLeft, toYUpLeft)) {
 availableMoves.add(Pair(toXUpLeft, toYUpLeft))
 }
 val toXUpRight = x - 1
 val toYUpRight = y + 1
 if (isValidPosition(toXUpRight, toYUpRight)) {
 availableMoves.add(Pair(toXUpRight, toYUpRight))
 }
 val toXDownLeft = x + 1
 val toYDownLeft = y - 1
 if (isValidPosition(toXDownLeft, toYDownLeft)) {
 availableMoves.add(Pair(toXDownLeft, toYDownLeft))
 }
 val toXDownRight = x + 1
 val toYDownRight = y + 1
 if (isValidPosition(toXDownRight, toYDownRight)) {
 availableMoves.add(Pair(toXDownRight, toYDownRight))
 }
 }
 Piece.Black.Pawn1, Piece.Black.Pawn2, Piece.Black.Pawn3, Piece.Black.Pawn4, Piece.Black.Pawn5, Piece.Black.Pawn6, Piece.Black.Pawn7, Piece.Black.Pawn8 -> {
 val toXDown = x + 1
 val toY1Down = y
 if (isValidPosition(toXDown, toY1Down) && !tileHasPieceOfOpponent(toXDown, toY1Down)) {
 availableMoves.add(Pair(toXDown, toY1Down))
 }
 val toXDownRight = x + 1
 val toYDownRight = y + 1
 if (isValidPosition(toXDownRight, toYDownRight)
 && tileHasPieceOfOpponent(toXDownRight, toYDownRight)
 ) {
 availableMoves.add(Pair(toXDownRight, toYDownRight))
 }
 val toXDownLeft = x + 1
 val toYDownLeft = y - 1
 if (isValidPosition(toXDownLeft, toYDownLeft)
 && tileHasPieceOfOpponent(toXDownLeft, toYDownLeft)
 ) {
 availableMoves.add(Pair(toXDownLeft, toYDownLeft))
 }
 }
 Piece.White.Pawn1, Piece.White.Pawn2, Piece.White.Pawn3, Piece.White.Pawn4, Piece.White.Pawn5, Piece.White.Pawn6, Piece.White.Pawn7, Piece.White.Pawn8 -> {
 val toXUp = x - 1
 val toYUp = y
 if (isValidPosition(toXUp, toYUp) && !tileHasPieceOfOpponent(toXUp, toYUp)) {
 availableMoves.add(Pair(toXUp, toYUp))
 }
 val toXUpRight = x - 1
 val toYUpRight = y + 1
 if (isValidPosition(toXUpRight, toYUpRight) && tileHasPieceOfOpponent(toXUpRight, toYUpRight)) {
 availableMoves.add(Pair(toXUpRight, toYUpRight))
 }
 val toXUpLeft = x - 1
 val toYUpLeft = y - 1
 if (isValidPosition(toXUpLeft, toYUpLeft) && tileHasPieceOfOpponent(toXUpLeft, toYUpLeft)) {
 availableMoves.add(Pair(toXUpLeft, toYUpLeft))
 }
 }
 }
 when (playingFieldArray[x][y]) {
 Piece.Black.Bishop1, Piece.Black.Bishop2, Piece.White.Bishop1, Piece.White.Bishop2, Piece.Black.Queen, Piece.White.Queen -> {
 var toXUpLeft = x - 1
 var toYUpLeft = y - 1
 while (isValidPosition(toXUpLeft, toYUpLeft)
 && !tileHasPieceOfCurrentPlayer(toXUpLeft, toYUpLeft)
 ) {
 availableMoves.add(Pair(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(Pair(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(Pair(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(Pair(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(Pair(toX, toY))) {
 if (tileHasPieceOfOpponent(toX, toY)) {
 removedPiecesList.add(playingField[toX][toY])
 }
 playingFieldArray[toX][toY] = playingFieldArray[fromX][fromY]
 playingFieldArray[fromX][fromY] = Piece.Empty
 } 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] is Piece.Black
 }
 Player.White -> {
 playingField[x][y] is Piece.White
 }
 }
 private fun tileHasPieceOfOpponent(x: Int, y: Int) = when (currentPlayer) {
 Player.Black -> {
 playingField[x][y] is Piece.White
 }
 Player.White -> {
 playingField[x][y] is Piece.Black
 }
 }
 fun isGameOver() =
 removedPieces.contains(Piece.White.King) || removedPieces.contains(Piece.Black.King)
 sealed class Piece {
 sealed class White : Piece() {
 object Pawn1 : White()
 object Pawn2 : White()
 object Pawn3 : White()
 object Pawn4 : White()
 object Pawn5 : White()
 object Pawn6 : White()
 object Pawn7 : White()
 object Pawn8 : White()
 object Rook1 : White()
 object Knight1 : White()
 object Bishop1 : White()
 object Queen : White()
 object King : White()
 object Bishop2 : White()
 object Knight2 : White()
 object Rook2 : White()
 }
 sealed class Black : Piece() {
 object Pawn1 : Black()
 object Pawn2 : Black()
 object Pawn3 : Black()
 object Pawn4 : Black()
 object Pawn5 : Black()
 object Pawn6 : Black()
 object Pawn7 : Black()
 object Pawn8 : Black()
 object Rook1 : Black()
 object Knight1 : Black()
 object Bishop1 : Black()
 object Queen : Black()
 object King : Black()
 object Bishop2 : Black()
 object Knight2 : Black()
 object Rook2 : Black()
 }
 object Empty : Piece()
 }
 enum class Player {
 Black, White
 }
}
```

• Heslacher: I have rolled back your latest edits. Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers .
• Florian Walther: @Heslacher sorry, I'll read the rules now!
• Florian Walther: new Question: codereview.stackexchange.com/questions/259851/chess-in-kotlin

Answer 1

There's several parts of your design that can be improved.

There is currently lots of duplicate code regarding color, allowed moves, and Pawn1, Pawn2, Pawn3 etc.

Is there really any difference between Pawn1, Pawn2, Pawn3, etc? I don't think so. A Piece could be a composition between a player color and an enum class PieceType that lists all the options (without numbering them).

Your move logic could use a List<Point> where a Point is a data class Point(val x: Int, val y: Int). For a knight this could then be:

val knightMoves = listOf(
 Point(2, 1), Point(2, -1),
 Point(-1, 2), Point(1, 2),
 Point(-1, -2), Point(1, -2),
 Point(-2, -1), Point(-2, 1)
)

Then iterate through this list and check which moves are allowed.

for (val move in knightMoves) {
 if (isValidPosition(x + move.x, y + move.y)) {
 availableMoves.add(move)
 }
}

It is possible to make more improvements, but... a few things at a time.

• Florian Walther: Wow, I just realized how bad my code actually is. I must've been too engrossed in getting it to work. Thank you, I will apply your suggestions tomorrow!
• Simon Forsberg: @FlorianWalther There's definitely room for improvement :) When you've made improvements, I'd recommend posting a follow-up question here on this site.
• Florian Walther: I am able to extract the Knight and King moves into a List<Point> as you described, but I don't see how I can do this for for example Rook moves, since the increment operator depends on the direction (like toXUp--) Any ideas?
• Florian Walther: I posted my new question here: codereview.stackexchange.com/questions/259851/chess-in-kotlin