loading...

Discussion on: Critique My Code: Sudoku Solver in Go

Collapse
bgadrian profile image
Adrian B.G.

I hate reading i/j and x/y loops, they don't mean anything, I suggest using meaningful names like "column, row".

Is no need to profile your code using start/end timings, there are many other ways (go profilers or a simple bash utilities like time go run ...) that don't require 'polutting the business logic'.

This is for Solve, getUnsolved, getNewUnsolved functions:

//for ...
if i == index {
                solved = append(solved[:j], solved[j + 1:]...)
                continue cellLoop
            }

In go is preferred to keep the "happy line" to the left and use quick exits, something like:

//for ...
if i != index {
continue 
}
                solved = append(solved[:j], solved[j + 1:]...)
                continue cellLoop

There is an optimal way to solve Sudokus, a smarter brute force using the Dancing Links method , although is more complicated to understand and code. There are also implementations on GitHub in C# and Java I think.

Collapse
mas profile image
Sam P Author

Thanks for your criticisms, I'll definitely take them into account. I agree with you that some of the loops aren't the most meaningful, and I'll clean that up (the i/j naming for the index in the loops is a habit I've carried over from Java where it's idiomatic).

I'll also make sure to check out that more optimal algorithm.

Collapse
bgadrian profile image
Adrian B.G.

Glad I could help, I love doing Go reviews, it helps me become a better dev.

Whenever you need a PR review just ping me, I'm a rookie to but I'll try my best.

PS: you can use the #reviews channel from the Gopher slack server too