[Haskell-cafe] Code Review: Sudoku solver

Robert Dockins robdockins at fastmail.fm
Wed Mar 22 16:24:12 EST 2006

On Mar 22, 2006, at 2:16 PM, David F. Place wrote:

> Hi All,
> I really appreciate all the help I received when I asked you to  
> critique my PrefixMap module a few weeks ago.  I think I am making  
> good progress in correcting the "lisp" in my Haskell programming.   
> I'll be very grateful to anyone who can take a glance at the  
> attached short program and say if any unidiomatic usages pop out.

> sudoku :: Sudoku -> IO ()
> sudoku s = ((mapM_ putStrLn) . (check s) . (take 1) . solveSudoku) s

> check puzzle [] = [showSudoku puzzle,"No solutions."]
> check puzzle [solution]
>       | solution `solves` puzzle =
>           ["Puzzle:",showSudoku puzzle,"Solution:",showSudoku  
> solution]
>       | otherwise = ["Program Error.  Incorrect Solution!"]

That '(check s) . (take 1)' bit looks a little odd to me.  I would  
simply have written 'check' to match like:

check puzzle [] = <failure case>
check puzzle (solution : _ ) = <success case>

Also, I like to put off doing IO as long as possible, so I'd probably  
have 'sodoku' return a String or [String] and move the putStr into  
main.  Its an easy way to make your code more reusable.

Also, your parser is pretty hackish (but I suspect you knew that  

FYI, solveSudoku has a bug; if you enter an invalid puzzle it will  
return non-solutions.

> It solves sudoku puzzles.  (What pleasure do people get by doing  
> these in their heads?!?)

I have no idea.

Rob Dockins

Speak softly and drive a Sherman tank.
Laugh hard; it's a long way to the bank.
           -- TMBG

More information about the Haskell-Cafe mailing list