Code Review for an Ordered Mistakes Feature

code_review.clj

(def test-state
  {:document {:paragraphs {:uuid-1 {}}
              :paragraph-order [:uuid-1 :uuid-2 :uuid-3 :uuid-4]}
   :edit {:spellcheck {:mistakes {:uuid-1 [[4 12] [38 42]]
                                  :uuid-3 [[7 12]]
                                  :uuid-4 []
                                  :uuid-X [[0 4]]}}}})

(defn paragraph-order [state]
  ;; get-in can be more readable than destructuring. Just know
  ;; that destructuring's an option. 
  (get-in state [:document :paragraph-order]))

(defn mistakes [state]
  ;; Unless you pull the `get-in` form from the `paragraph-order` function,
  ;; you could just call the `paragraph-order` function in the `some` function.
  (let [paragraph-order (paragraph-order state)]
    ;; Thread-last macro for readability, nice :) 
    (->> (get-in state [:edit :spellcheck :mistakes]) 
         ;; _only paragraphs with mistakes and present on paragraph-order_ [nice comment]
         ;; whenever you see `filter`, `map`, etc. you should think "transducers".
         ;; they're harder to get at first, but they offer enormous performance boosts.
         (filter (fn [[k v]] (and (seq v) (some #{k} paragraph-order))))))) ;; nice use of `some` with the set of k

(defn ordered-mistakes [state]
  (let [paragraph-order (paragraph-order state)] ;; this is repetitive... 
    (->> (mistakes state) ;; `mistakes` is thread-lasting like this one - why not combine?
         ;; _sort-by paragraph-order_
         ;; you could delete the `let` and just call `(paragraph-order state)` here.
         (sort-by (fn [[k _]] (.indexOf paragraph-order k))) 
         ;; _"unpack" into [:p-uuid [word-start word-end]]_ [nice comment - wasn't obvious what this did at first]
         (mapcat (fn [[k v]] (map (fn [i] [k i]) v)))
         ;; Vectors are nice if you plan to reverse or work with the last elements.
         ;; They also look nicer and are conceptually easier. There's a small chance
         ;; turning it into a vector may be unnecessary here. 
         (vec))))

;; Overall notes:
;;  - It works. Nice job :D
;;  - Good readability due to thread-last macro-ing and clear comments
;;  - The first two functions are conceptually simple and similar enough 
;;    to combine into the last. 
;;  - Filtering and mapping should prompt "transducers" into your mind.
;;    `filter` and` `map` (`mapcat`) are "reducing" functions. 
;;    Transducers can combine multiple of these into one reducing function.
;;    When I wrote my code for this problem, I improved performance by 4.6 times, 
;;    requiring <22% of the time that this code requires.


(comment (ordered-mistakes test-state))