[Haskell-cafe] Code Review: Sudoku solver

[Robert Dockins]

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  
already).


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