From 2ff7d1759e9a6bc1378f9a9f00b231b61ea04aac Mon Sep 17 00:00:00 2001 From: lila Date: Tue, 28 Apr 2026 13:17:24 +0200 Subject: [PATCH] refactor: extract shuffleArray to lib/utils, rename correctAnswers to terms --- apps/api/src/lib/utils.ts | 10 ++++++++ apps/api/src/services/gameService.ts | 30 ++++++++-------------- documentation/backlog.md | 4 +-- documentation/roasts/gameService.md | 38 ---------------------------- documentation/tickets/t00003.md | 37 +++++++++++++++++++++++++++ 5 files changed, 59 insertions(+), 60 deletions(-) create mode 100644 apps/api/src/lib/utils.ts create mode 100644 documentation/tickets/t00003.md diff --git a/apps/api/src/lib/utils.ts b/apps/api/src/lib/utils.ts new file mode 100644 index 0000000..4912c8c --- /dev/null +++ b/apps/api/src/lib/utils.ts @@ -0,0 +1,10 @@ +export const shuffleArray = (array: T[]): T[] => { + const result = [...array]; + for (let i = result.length - 1; i > 0; i--) { + const j = Math.floor(Math.random() * (i + 1)); + const temp = result[i]!; + result[i] = result[j]!; + result[j] = temp; + } + return result; +}; diff --git a/apps/api/src/services/gameService.ts b/apps/api/src/services/gameService.ts index d0f0781..64f90f6 100644 --- a/apps/api/src/services/gameService.ts +++ b/apps/api/src/services/gameService.ts @@ -10,13 +10,14 @@ import type { } from "@lila/shared"; import { InMemoryGameSessionStore } from "../gameSessionStore/index.js"; import { NotFoundError } from "../errors/AppError.js"; +import { shuffleArray } from "../lib/utils.js"; const gameSessionStore = new InMemoryGameSessionStore(); export const createGameSession = async ( request: GameRequest, ): Promise => { - const correctAnswers = await getGameTerms( + const terms = await getGameTerms( request.source_language, request.target_language, request.pos, @@ -27,19 +28,19 @@ export const createGameSession = async ( const answerKey = new Map(); const questions: GameQuestion[] = await Promise.all( - correctAnswers.map(async (correctAnswer) => { + terms.map(async (term) => { const distractorTexts = await getDistractors( - correctAnswer.termId, - correctAnswer.targetText, + term.termId, + term.targetText, request.target_language, request.pos, request.difficulty, 3, ); - const optionTexts = [correctAnswer.targetText, ...distractorTexts]; - const shuffledTexts = shuffle(optionTexts); - const correctOptionId = shuffledTexts.indexOf(correctAnswer.targetText); + const optionTexts = [term.targetText, ...distractorTexts]; + const shuffledTexts = shuffleArray(optionTexts); + const correctOptionId = shuffledTexts.indexOf(term.targetText); const options: AnswerOption[] = shuffledTexts.map((text, index) => ({ optionId: index, @@ -51,8 +52,8 @@ export const createGameSession = async ( return { questionId, - prompt: correctAnswer.sourceText, - gloss: correctAnswer.sourceGloss, + prompt: term.sourceText, + gloss: term.sourceGloss, options, }; }), @@ -64,17 +65,6 @@ export const createGameSession = async ( return { sessionId, questions }; }; -const shuffle = (array: T[]): T[] => { - const result = [...array]; - for (let i = result.length - 1; i > 0; i--) { - const j = Math.floor(Math.random() * (i + 1)); - const temp = result[i]!; - result[i] = result[j]!; - result[j] = temp; - } - return result; -}; - export const evaluateAnswer = async ( submission: AnswerSubmission, ): Promise => { diff --git a/documentation/backlog.md b/documentation/backlog.md index 4e6af84..d0cb202 100644 --- a/documentation/backlog.md +++ b/documentation/backlog.md @@ -103,10 +103,10 @@ Directionally right, timing is unclear. Revisit when the next/now work is done. Shipped milestones, newest first. - **04 - 2026 - t00001 - Docker credential helper** -- **04 - 2026 - Pin dependencies in package.json** - Unpinned deps in a CI/CD pipeline are a real risk. +- **04 - 2026 - Pin dependencies in package.json** - Unpinned deps in a CI/CD pipeline are a real risk. - **04 - 2026 - React error boundaries** - Catch and display runtime errors gracefully instead of crashing the entire app. - **04 - 2026 - 404 and redirect handling** - Unknown routes return raw errors. Add a catch-all route on the frontend for client-side 404s. -- **04 - 2026 - Multiplayer GameService unit tests** - round evaluation, scoring, tie-breaking, timeout handling +- **04 - 2026 - Multiplayer GameService unit tests** - round evaluation, scoring, tie-breaking, timeout handling - **04 - 2026 - Security headers with helmet** - Add helmet middleware to set secure HTTP response headers. - **04 - 2026 - Rate limiting on API endpoints** - At minimum: auth endpoints (brute force prevention) and game endpoints (spam prevention) - **04 - 2026 — Migrations in deploy pipeline** — Drizzle migrate runs as a CI/CD step before the API container restarts diff --git a/documentation/roasts/gameService.md b/documentation/roasts/gameService.md index e5663b5..de8f968 100644 --- a/documentation/roasts/gameService.md +++ b/documentation/roasts/gameService.md @@ -115,42 +115,6 @@ export class InMemoryGameSessionStore implements GameSessionStore { --- -## 3. `shuffle` is defined after it's used - -**Problem** - -`shuffle` is called inside `createGameSession` but defined below it. It works at runtime (module evaluation order), but reads as if the file was written top-to-bottom without a plan. - -```ts -// ❌ shuffle appears after the function that calls it -export const createGameSession = async (...) => { - const shuffledTexts = shuffle(optionTexts); // used here -}; - -const shuffle = (array: T[]): T[] => { ... }; // defined down here -``` - -**Fix — move helpers to the top, exports to the bottom** - -```ts -// ✅ utilities first, then exported functions -const shuffle = (array: T[]): T[] => { - const result = [...array]; - for (let i = result.length - 1; i > 0; i--) { - const j = Math.floor(Math.random() * (i + 1)); - const temp = result[i]!; - result[i] = result[j]!; - result[j] = temp; - } - return result; -}; - -export const createGameSession = async (...) => { ... }; -export const evaluateAnswer = async (...) => { ... }; -``` - ---- - **Problem** @@ -181,8 +145,6 @@ The `z.coerce.number()` handles the case where the value arrives as a string fro --- -## 5. `correctAnswers` is a misleading variable name - **Problem** The variable holds `terms` — word pairs fetched from the database. Calling them `correctAnswers` jumps ahead semantically; they only become "correct answers" once options are constructed around them. diff --git a/documentation/tickets/t00003.md b/documentation/tickets/t00003.md new file mode 100644 index 0000000..774fe42 --- /dev/null +++ b/documentation/tickets/t00003.md @@ -0,0 +1,37 @@ +# refactor: extract shuffleArray to lib/utils, rename correctAnswers to terms + +## Problem + +Two readability issues in `gameService.ts`: + +1. `shuffle` was defined as a private function at the bottom of `gameService.ts`, after the function that calls it. It is a pure generic utility with no dependency on game domain logic, so it had no business living there. + +2. The variable holding terms fetched from the database was named `correctAnswers`. These are word pairs — they only become "correct answers" once options are built around them. The name was premature and misleading. + +## Options considered + +### Option A — Move `shuffle` up in the same file + +Simple, no new files. Fixes the ordering issue but keeps a generic utility buried in domain code. + +### Option B — Extract to `lib/utils.ts` ✅ + +Move `shuffle` (renamed `shuffleArray`) to `apps/api/src/lib/utils.ts` and import it. Cleaner separation: domain logic stays in services, generic utilities live in `lib/`. + +Chosen because `lib/` already exists, the function is reusable, and it gives future utilities a home. + +## Solution + +- Created `apps/api/src/lib/utils.ts` with `shuffleArray` +- Renamed `shuffle` → `shuffleArray` for clarity at the call site +- Removed the inline `shuffle` from `gameService.ts` and imported from `lib/utils.ts` +- Renamed `correctAnswers` → `terms` and `correctAnswer` → `term` throughout `gameService.ts` + +## Files changed + +- `apps/api/src/lib/utils.ts` — created +- `apps/api/src/services/gameService.ts` — removed `shuffle`, updated import, renamed variables + +## Commit + +`refactor: extract shuffleArray to lib/utils, rename correctAnswers to terms`