code-reviews 2020-08-27 | Slack Archive

Hey teams, one beginner code review:

(defn print-positions [board-size positions]
  (println "---------")
  (->> (range board-size)
       (map (fn [x]
              (map (fn [y]
                     (if (positions [x y])
                       "X"
                       "_"))
                   (range board-size))))
       (run! println)))

Effectively, given a "board-size", and a set of positions, print a nice representation with those positions ^ Would you write this differently?

jaihindhreddy18:08:29

Instead of using range and processing indices, I'd make the matrix first, using repeat, then fill in the positions with reduce and assoc-in, and then make the right strings for each line using map and finally run! println to print it:

(defn print-positions [n positions]
  (let [matrix (vec (repeat n (vec (repeat n \_))))
        filled (reduce #(assoc-in % %2 \X) matrix positions)]
    (->> (map #(str/join \space %) filled)
         (run! println))))

And an example call:

user=> (print-positions 10 #{[0 0] [1 1] [3 3]})
X _ _ _ _ _ _ _ _ _
_ X _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _
_ _ _ X _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _
nil
user=>

Reads more linearly this way.

❤️ 3

stopa15:08:01

(Also an aside: was very confused by print. Sometimes if I wrote print , it would not show up in the repl. Also I couldn't seem to get print to "print out" strings with newlines in them, represented at actual new lines. Will need to look deeper, but if you have thoughts / resources on how to best print in clj, lmk : })

TMac16:08:55

TBH, I feel like wrapping inherently side-effecting code in functional constructs can get a little cumbersome. I’d just go imperative and use a doseq:

(defn print-positions2
        [occupied? board-size]
        (println (str/join (repeat 10 "-")))
        (doseq [y (range board-size)
                x (range board-size)]
          (print (if (occupied? [x y]) "X" "_"))
          (when (= x (dec board-size))
            (println))))

(worth noting that someone could probably one-line this with cl-format, but this is the Clojurians Slack, not the Common Lispers one)

stopa18:08:35

Oo very nice ideas. Will also look deeper on cl-format! Thanks @jaihindhreddy @timgilbert

⁉️ 3

phronmophobic18:08:25

I think splitting it into two doseqs makes it slightly more readable, but I agree with @tsmacdonald that doseq is a better fit for side-effecting code:

(defn print-positions2
  [occupied? board-size]
  (println (str/join (repeat 10 "-")))
  (doseq [y (range board-size)]
    (doseq [x (range board-size)]
      (print (if (occupied? [x y]) "X" "_")))
    (println)))

❤️ 3

stopa19:08:34

Nice, thanks team!

stopa19:08:18

Okay, one more -- this time a bit more involved: This is the code to encode a message using a hoffman tree (@joeaverbukh and I are going through SICP together xD) [1]

(defn encode [tree message]
  (letfn [(encode-symbol-helper [symbol path branch]
            (cond
              (= [symbol] (symbols branch))
              path

              :else
              (let [left (left-branch branch)
                    right (right-branch branch)]
                (or
                  (and left (encode-symbol-helper symbol (conj path 0) left))
                  (and right (encode-symbol-helper symbol (conj path 1) right))))))
          (encode-symbol [symbol]
            (if-let [res (encode-symbol-helper symbol [] tree)]
              res
              (throw (Exception. (format "Uh oh, couldn't find symbol = %s" symbol)))))]
    (mapcat encode-symbol message)))

(comment
  (def sample-tree (make-code-tree (make-leaf 'A 4)
                                   (make-code-tree (make-leaf 'B 2) (make-code-tree
                                                                      (make-leaf 'D 1)
                                                                      (make-leaf 'C 1)))))
  (def sample-bits '(0 1 1 0 0 1 0 1 0 1 1 1 0))
  (def sample-message '(A D A B B C A))
  (= (encode sample-tree sample-message) sample-bits))

Idea: is, for each symbol, we dfs over the tree, and return the path which leads to the correct leaf node. We throw if any errors If can be written even more clojure-style or consicely lmk : } [1] full code https://github.com/nezaj/clj-sicp/blob/master/src/chp2.clj#L230-L253

TMac19:08:44

I haven’t had enough coffee to talk about the algorithm, but the cond can be replaced by an if and encode-symbol-helper can be replaced by providing multiple arities to encode (something like this:)

(defn encode
  ([tree message]
    (letfn [(encode-symbol …]
     (mapcat encode-symbol message)))
  ([symbol path branch]
    (if (= [symbol] …)))

❤️ 3

stopa20:08:11

Great ideas! And nice profile pic @tsmacdonald! : )

seancorfield21:08:07

I would suggest using sym as the local binding so you don't shadow symbol in clojure.core -- I found the code hard to read because of that (I kept thinking you were referring to clojure.core/symbol in various calls).

❤️ 3

stopa21:08:38

Oi great point!

2020-08-27

Channels