From 8b235a25e01fb74c58df7a9565ee6a93cb84c3c1 Mon Sep 17 00:00:00 2001 From: Janis Date: Tue, 24 Mar 2026 18:24:36 +0100 Subject: [PATCH] docs: add functional style refactor design spec --- ...-03-24-functional-style-refactor-design.md | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 docs/superpowers/specs/2026-03-24-functional-style-refactor-design.md diff --git a/docs/superpowers/specs/2026-03-24-functional-style-refactor-design.md b/docs/superpowers/specs/2026-03-24-functional-style-refactor-design.md new file mode 100644 index 0000000..e95e4b4 --- /dev/null +++ b/docs/superpowers/specs/2026-03-24-functional-style-refactor-design.md @@ -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.