[Haskell-cafe] Code Review: Sudoku solver
haskell at list.mightyreason.com
Wed Mar 22 17:12:50 EST 2006
Robert Dockins wrote:
> 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.
The style of the code and choice of names is good.
>> 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.
> 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>
A simpler version of replace:
replace :: Sudoku -> Int -> Int -> Int -> Sudoku
replace s r c x =
let (above,row:below) = splitAt r s
(left,_:right) = splitAt c row
And a simpler version of toList in keeping with your style:
toList :: Set -> [Int]
toList i = concatMap f [9,8..1]
f b = if testBit i b then [b] else 
(The above is also a little less prone to off-by-one errors since testBit is the
opposite of setBit)
> 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).
A minimal change to the parser that still does no sanity checking but may be a
little more robust is
readSudoku :: [String] -> String -> Sudoku
readSudoku ["line"] input =
takeBy 9 $ map digitToInt $ filter isDigit $ head $ lines input
readSudoku _ input =
map (map digitToInt . filter isDigit) $ lines input
More information about the Haskell-Cafe