HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Sudoku solver in Racket

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
solversudokuracket

Problem

I have written following code to solve Sudoku puzzles in Racket (a Scheme/Lisp derivative programming language). Following relatively simple code appears to work but are there any bugs in it or can it be further optimized? (semi-colon indicates start of comment on that line).

(define (SolveSudoku sentboard)
  (define (subgrid brd r c)
    (define ll '((0 1 2)(3 4 5)(6 7 8)))
    (define-values (rr cc)
      (values (flatten (filter (λ (x) (member r x)) ll))
              (flatten (filter (λ (x) (member c x)) ll))))
    (for*/list ((i rr)(j cc))
      (list-ref (list-ref brd i) j)))
  (let loop ((bd sentboard) (r 0) (c 0))
    (cond [(= 0 (list-ref (list-ref bd r) c) )
           (for ((i (range 1 10)))
             (when (and (not(member i (list-ref bd r)))                 
                        (not(member i (map (λ (x) (list-ref x c)) bd))) 
                        (not(member i (subgrid bd r c))))                
               (define newbd (list-set bd r
                                       (list-set (list-ref bd r) c i)))
               (cond [(< (add1 c) 9)
                      (loop newbd r (add1 c) )]
                     [(< (add1 r) 9)
                      (loop newbd (add1 r) 0 )]
                     [else (displayln "SOLUTION:")
                           (for ((rowline newbd)) (println rowline))])))]
          [(< (add1 c) 9)
           (loop bd r (add1 c))]
          [(< (add1 r) 9)
           (loop bd (add1 r) 0)]
          [else (displayln "Solution:")  
                (for ((rowline bd)) (println rowline))])))


Following is code with comments added:

```
(define (SolveSudoku sentboard)
; subfn to get list of numbers in 3x3 subgrid for a given row and column position:
(define (subgrid brd r c)
(define ll '((0 1 2)(3 4 5)(6 7 8)))
; get row/column set to which r and c belong:
(define-values (rr cc)
(values (flatten (filter (λ (x) (member r x)) ll))
(flatten (filter (λ (x) (member c x)) ll))))

Solution

This looks like a fairly reasonable brute force method. From what I've gathered though, the for loop will try for all combination rather than just the first valid one. However the for will sequence the operation to avoid a huge memory overhead.

However if the puzzle is not well constructed it may have more than one valid solution, and I suspect this code will print them both. https://sandwalk.blogspot.com/2007/06/i-knew-it-there-can-be-more-than-one.html

A custom loop that breaks when a solution is found would on average run in half the time, and never give you more than one solution. But I'm not going to show you how to do it, since there are several solutions, and it's not a huge improvement.

A few minor things.

[(< (add1 c) 9)
   (loop bd r (add1 c))]


And similar lines could easily be rewritten like

[(< c 8)       (loop bd r (add1 c))]


(and (not A) (not B) ...) is more simply written as '(not (or A B ...))

And a little more DSL would make the code more self-documenting.

And notice how you have almost exactly the same (cond ...)in two different locations, and the only difference is the next board you pass along. A good candidate for a little more abstraction.

(define (SolveSudoku sentboard)
  ;Seting up the DSL
  (define (row brd r) (list-ref brd r))
  (define (column brd c) (map (λ (r) (list-ref r c)) brd))
  (define (subgrid brd r c)
    (define ll '((0 1 2)(3 4 5)(6 7 8))) ;')
    (define-values (rr cc)
      (values (flatten (filter (λ (x) (member r x)) ll))
              (flatten (filter (λ (x) (member c x)) ll))))
    (for*/list ((i rr)(j cc))
      (list-ref (list-ref brd i) j)))
  (define (cell brd r c)  (list-ref (list-ref brd r) c))
  (define (new-board-with-updated-value bd r c val)
     (list-set bd r (list-set (list-ref bd r) c val)))
  (let loop ((bd sentboard) (r 0) (c 0)) ;initialize counters
       (let ((next (lambda (nextbd) ;the next step through the board
                     (cond [(< c 8)  (loop nextbd r (add1 c) )]
                           [(< r 8)  (loop nextbd (add1 r) 0 )]
                           [else (displayln "SOLUTION:")
                                 (for ((rowline nextbd)) 
                                       (println rowline))]))))
         (if (= 0 (cell bd r c))
             (for ((i (range 1 10)))
;brute force the solution, with a simple collision check first
                   (when (not (or (member i (row     bd r))                 
                                  (member i (column  bd c)) 
                                  (member i (subgrid bd r c))))              
                      (next (new-board-with-updated-value bd r c i))))
                 (next bd)))))


Now I know I've messed up brackets or parenthesis somewhere, but I hope I've given you and idea on how much cleaner the code can be. And now as a bonus, because I've abstracted how the loop interacts with the data, you are now able to replace the list of lists data structure with a better data structure. Not that it would make a huge difference for a data set this small, but could give you order of magnitude savings on different sorts of problems.

Code Snippets

[(< (add1 c) 9)
   (loop bd r (add1 c))]
[(< c 8)       (loop bd r (add1 c))]
(define (SolveSudoku sentboard)
  ;Seting up the DSL
  (define (row brd r) (list-ref brd r))
  (define (column brd c) (map (λ (r) (list-ref r c)) brd))
  (define (subgrid brd r c)
    (define ll '((0 1 2)(3 4 5)(6 7 8))) ;')
    (define-values (rr cc)
      (values (flatten (filter (λ (x) (member r x)) ll))
              (flatten (filter (λ (x) (member c x)) ll))))
    (for*/list ((i rr)(j cc))
      (list-ref (list-ref brd i) j)))
  (define (cell brd r c)  (list-ref (list-ref brd r) c))
  (define (new-board-with-updated-value bd r c val)
     (list-set bd r (list-set (list-ref bd r) c val)))
  (let loop ((bd sentboard) (r 0) (c 0)) ;initialize counters
       (let ((next (lambda (nextbd) ;the next step through the board
                     (cond [(< c 8)  (loop nextbd r (add1 c) )]
                           [(< r 8)  (loop nextbd (add1 r) 0 )]
                           [else (displayln "SOLUTION:")
                                 (for ((rowline nextbd)) 
                                       (println rowline))]))))
         (if (= 0 (cell bd r c))
             (for ((i (range 1 10)))
;brute force the solution, with a simple collision check first
                   (when (not (or (member i (row     bd r))                 
                                  (member i (column  bd c)) 
                                  (member i (subgrid bd r c))))              
                      (next (new-board-with-updated-value bd r c i))))
                 (next bd)))))

Context

StackExchange Code Review Q#151749, answer score: 2

Revisions (0)

No revisions yet.