-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Doding dojo JS edition - Sudoku solver Kata. Artur Wodarz solution #1
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for sending your solution.
function parseToArray(sudokuAsString) { | ||
let sudokuArray = helper.createMatrix(9); | ||
sudokuAsString.split("").forEach((el, index) => { | ||
sudokuArray[Math.floor(index / 9)][index % 9] = el != "_" ? parseInt(el) : null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This Math.floor(index / 9)[index %9]
is clever 😃
but it also is not tested. Since this is the smartest part of the whole function I'd move this calculation into a separate function that returns a tuple.
function arrayPosition(index: number): [number, number];
|
||
describe("Sudoku one", function() { | ||
it("should be solved", function() { | ||
var unsolved_sudoku = "___26_7_168__7__9_19___45__82_1___4___46_29___5___3_28__93___74_4__5__367_3_18___"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a small inconsistency. Using linter and formatter may help you with these.
if (sudokuMatrix[x][y] == null) sudokuMatrix[x][y] = 0; | ||
let newUnique = helper.findUniqueInt( | ||
sudokuMatrix[x][y], | ||
9, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a magic number. It also can be retrieved from parsed matrix dimension.
const parser = require("./sudokuParser"); | ||
const helper = require("./helper"); | ||
|
||
function getRelatedCells(cell, sudokuMatrix) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When reading body of solve
function my initial though when reached line with getRelatedCells
was to look into helper
module. I was wrong 😄 Maybe you should move it there.
while (helper.inRange(0, modifiableCells.length - 1, nowModified)) { | ||
let x = modifiableCells[nowModified].row; | ||
let y = modifiableCells[nowModified].col; | ||
if (sudokuMatrix[x][y] == null) sudokuMatrix[x][y] = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you only set the cell to 0
so helper.findUniqueInt
can work. You should move this if into helper function.
Separate functions should not be aware of such details.
return block; | ||
} | ||
|
||
function extractRow(rowIndex, matrix) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You missed unit tests for this function.
No description provided.