Sudoku review #1

Open
BordedDev wants to merge 19 commits from BordedDev/sudoku:main into main
pull from: BordedDev/sudoku:main
merge into: retoor:main
Conversation 3 Commits 19 Files Changed 8 +2 -2
1 changed files with 2 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit a9d1fd2c62 - Show all commits

4
reviews/BordedDev/sudoku.rewrite.js
View File

@ -30,7 +30,7 @@ class SudokuPuzzle extends EventTarget {
* *
* @type {(number|null|undefined)[]} * @type {(number|null|undefined)[]}
*/ */
BordedDev commented 2025-01-16 23:51:24 +00:00
Outdated
Review
Copy Link

This contains a stack of states, essentially the initial state + all steps

This contains a stack of states, essentially the initial state + all steps
#activeState = new Array(9 * 9) #activeState = new Array(SUDOKU_GRID_SIZE)
retoor commented 2025-01-16 18:39:29 +00:00
Outdated
Review
Copy Link

Ooooh, magic numbers :P Did i have that too?

Ooooh, magic numbers :P Did i have that too?
BordedDev commented 2025-01-16 21:26:53 +00:00
Outdated
Review
Copy Link

Dang it I missed it, it's meant to be SUDOKU_GRID_SIZE. You did with the new Puzzle(9)

Dang it I missed it, it's meant to be `SUDOKU_GRID_SIZE`. You did with the `new Puzzle(9)`
/** /**
* *
@ -61,7 +61,7 @@ class SudokuPuzzle extends EventTarget {
/** /**
* *
* @param serializedState {string} * @param serializedState {string}
* @returns {(?string)[]} * @returns {((number|null|undefined))[]}
*/ */
static #stateFromString(serializedState) { static #stateFromString(serializedState) {
let cellIndex = 0 let cellIndex = 0