Fork me on GitHub
#code-reviews
<
2015-09-03
>
sooheon12:09:40

I’m trying to learn by translating some of Peter Norvig’s python (http://norvig.com/sudoku.html) to clojure. Could anyone point out what I could do better here?

``````(ns sudoku.core
(:require [clojure.set :refer [difference]]))

(defn cross
"Cross product of elements in A and elements in B."
[a-list b-list]
(for [a a-list
b b-list]
(str a b)))

(def rows "ABCDEFGHI")

(def cols "123456789")

(def squares
(cross rows cols))

(def unitlist
(concat (for [c cols] (->> (cross rows (repeat 9 c)) set vec sort))
(for [r rows] (->> (cross (repeat 9 r) cols) set vec sort))
(for [rs ["ABC" "DEF" "GHI"]
cs ["123" "456" "789"]]
(->> (cross rs cs) vec))))

(def units
"Hash-map with squares as keys and the units of each square as a list of
lists."
(into {} (for [s squares]
[s (for [u unitlist :when (some #{s} u)]
u)])))

(def peers
"Hash-map associating each square and its peers."
(into {} (for [s squares]
[s (-> (units s)
flatten
set
(difference #{s}))])))
``````

meow13:09:18

@sooheon: ah, interesting. I am quite familiar with Norvig's sudoku code as I rewrote a variation of it to go with a QML Sudoku app I wrote.

sooheon13:09:33

@meow: Hey it’s been a while! Spent the summer doing other stuff but back to learning clojure again :) I also have the test code

``````(deftest sudoku-setup
(testing "Basic unit tests for the board."
(is (= 81 (count squares)))
(is (= 27 (count unitlist)))
(is (every? #(= 3 %) (for [s squares]
(count (units s)))))
(is (every? #(= 20 %) (for [s squares]
(count (peers s)))))
(is (= [["A2" "B2" "C2" "D2" "E2" "F2" "G2" "H2" "I2"]
["C1" "C2" "C3" "C4" "C5" "C6" "C7" "C8" "C9"]
["A1" "A2" "A3" "B1" "B2" "B3" "C1" "C2" "C3"]]
(units "C2")))
(is (= #{"I2" "A1" "E2" "A3" "C1" "B1" "B3" "C3" "C7"
"A2" "C6" "G2" "C5" "F2" "B2" "C8" "C4" "H2"
"D2" "C9"}
(peers "C2")))))
``````

sooheon13:09:08

Nice, I can look through both yours and Norvigs version

meow13:09:05

Norvig's version is slightly flawed in that it will produce puzzles that can have more than one valid solution, which is usually NOT what you want from a sudoku generator

meow13:09:18

My version fixed that

meow13:09:36

But my code reflects my then infatuation with OO code.

meow13:09:54

I hesitate to really look at it because I'm sure I'll hate it now.

meow13:09:34

The QML app I created was quite nice, if I do say so myself. Unfortunately, it convinced me that QML was not what I wanted to work with long term and then I moved on to Clojure and I am so glad that I did.

meow13:09:08

I wrote a fair amount of unit tests for that Python code: https://github.com/pkobrien/sudoku/blob/master/tests/test_sudoku.py

sooheon13:09:18

Wow this is all very helpful, all the differences/improvements to Norvig’s code will be great to study.

sooheon13:09:23

Anything about my clojure code to critique? I wonder if there’s any better way to do the (is (every? #(= 3 %) (foo))) test...

meow13:09:27

I haven't done that much unit testing in clojure (something I'm just now working on) but those tests look good to me.

potetm13:09:54

I would tend to do `(every? #{3} ..)`.

potetm13:09:39

But there are some tradeoffs there I presume. I would assume `(partial = 3)` to be more performant, but, in my mind, the “set of 3” allows you to think more about data and less about the particular operation you want to perform (i.e. `=`).

sooheon14:09:04

@potetm: Thanks, the set notation makes for much more readable reports than anonymous functions when the test fails as well.