docs: add functional style refactor design spec

2026-03-24 18:24:36 +01:00
parent b4116e9a82
commit 8b235a25e0
1 changed files with 71 additions and 0 deletions
@@ -0,0 +1,71 @@
+# Functional Style Refactor Design
+
+**Date:** 2026-03-24
+**Scope:** `MoveValidator.castlingTargets`, `Renderer.render`
+
+## Problem
+
+Two functions contain imperative anti-patterns that violate the project's Scala 3 functional style:
+
+1. `MoveValidator.castlingTargets` — uses `return` (early exits) and `var result` with `result +=` mutation.
+2. `Renderer.render` — uses a mutable `StringBuilder` with nested imperative `for` loops.
+
+## Design
+
+### 1. `MoveValidator.castlingTargets` (`modules/core/.../logic/MoveValidator.scala`)
+
+**Change:** Replace the two `return Set.empty` guards with a single `if/else` expression at the top. Replace `var result` + `result +=` with two `Option.when(...)(...).toSet` values combined with `++`.
+
+```scala
+def castlingTargets(ctx: GameContext, color: Color): Set[Square] =
+  val rights = ctx.castlingFor(color)
+  val rank   = if color == Color.White then Rank.R1 else Rank.R8
+  val kingSq = Square(File.E, rank)
+  val enemy  = color.opposite
+
+  if !ctx.board.pieceAt(kingSq).contains(Piece(color, PieceType.King)) ||
+     GameRules.isInCheck(ctx.board, color) then Set.empty
+  else
+    val kingsideSq = Option.when(
+      rights.kingSide &&
+      ctx.board.pieceAt(Square(File.H, rank)).contains(Piece(color, PieceType.Rook)) &&
+      List(Square(File.F, rank), Square(File.G, rank)).forall(s => ctx.board.pieceAt(s).isEmpty) &&
+      !List(Square(File.F, rank), Square(File.G, rank)).exists(s => isAttackedBy(ctx.board, s, enemy))
+    )(Square(File.G, rank))
+
+    val queensideSq = Option.when(
+      rights.queenSide &&
+      ctx.board.pieceAt(Square(File.A, rank)).contains(Piece(color, PieceType.Rook)) &&
+      List(Square(File.B, rank), Square(File.C, rank), Square(File.D, rank)).forall(s => ctx.board.pieceAt(s).isEmpty) &&
+      !List(Square(File.D, rank), Square(File.C, rank)).exists(s => isAttackedBy(ctx.board, s, enemy))
+    )(Square(File.C, rank))
+
+    kingsideSq.toSet ++ queensideSq.toSet
+```
+
+### 2. `Renderer.render` (`modules/core/.../view/Renderer.scala`)
+
+**Change:** Replace `StringBuilder` + nested `for` loops with nested `map`/`mkString`.
+
+```scala
+def render(board: Board): String =
+  val rows = (0 until 8).reverse.map { rank =>
+    val cells = (0 until 8).map { file =>
+      val sq        = Square(File.values(file), Rank.values(rank))
+      val isLightSq = (file + rank) % 2 != 0
+      val bgColor   = if isLightSq then AnsiLightSquare else AnsiDarkSquare
+      board.pieceAt(sq) match
+        case Some(piece) =>
+          val fgColor = if piece.color == Color.White then AnsiWhitePiece else AnsiBlackPiece
+          s"$bgColor$fgColor ${piece.unicode} $AnsiReset"
+        case None =>
+          s"$bgColor   $AnsiReset"
+    }.mkString
+    s"${rank + 1} $cells ${rank + 1}"
+  }.mkString("\n")
+  s"  a  b  c  d  e  f  g  h\n$rows\n  a  b  c  d  e  f  g  h\n"
+```
+
+## Testing
+
+No new tests needed — existing tests cover both functions. Run `./gradlew :modules:core:test` to verify green build after changes.