Write clear Clojure code with let

..

in praise of imperative Clojure code with let that does what says it does

For a long time, I’ve felt bad for writing imperative code in Clojure. Functional looks better! Is it my fault for not seeing the inherently functional nature of the I’m problem I’m working on?

Clojure code should be pretty! It should be functional! Clojure code should have the clarity of angels singing!

Then, I sit down with my editor, and I feel kind of stressed. I’ve got an JSON HTTP payload, and I need to deliver another JSON HTTP payload. It’s a pile of assignment. The code does not sing. It barfs, slightly off-tune.

Can I make it sing?


A series of events have lead me to spend some time reading Michiel Borkent’s code lately. Here’s the source for bb print-deps, edited to fit on this page:

(defn print-deps [deps-format]
  (let [deps (-> (io/resource "META-INF/babashka/deps.edn")
                 slurp
                 edn/read-string)
        deps (:deps deps)
        deps (assoc
              deps
              'babashka/fs {:mvn/version "0.4.19"}
              'babashka/babashka.curl {:mvn/version "0.1.2"}

              'babashka/babashka.core
              {:git/url "https://github.com/babashka/babashka.core"
               :git/sha "52a6037bd4b632bffffb04394fb4efd0cdab6b1e"}

              'babashka/process {:mvn/version "0.5.21"})
        deps (dissoc
              deps
              'borkdude/sci
              'org.babashka/sci
              'borkdude/graal.locking
              'org.postgresql/postgresql
              'babashka/clojure-lanterna
              'seancorfield/next.jdbc
              'datascript/datascript
              'org.hsqldb/hsqldb)
        bb-edn-deps (:deps @common/bb-edn)
        deps (merge deps bb-edn-deps)
        paths (:paths @common/bb-edn)
        deps {:deps deps}
        deps (cond-> deps
               (seq paths) (assoc :paths paths))]
    (case deps-format
      ("deps" nil) (binding [*print-namespace-maps* false]
                     (pp/pprint deps))
      ("classpath") (let [cp (str/trim
                              (sci/with-out-str
                                (deps/clojure
                                 ["-Spath" "-Sdeps" deps]
                                 {:out :string})))]
                      (println cp)))))

Source.

Here’s another Michiel Borkent excerpt, this time from neil dep add. Again, edited to fit here.

(defn dep-add [{:keys [opts]}]
  (if (or (:help opts) (:h opts) (not (:lib opts)))
    (print-dep-add-help)
    (do
      (ensure-deps-file opts)
      (let [edn-string (edn-string opts)
            edn-nodes (edn-nodes edn-string)
            lib (:lib opts)
            lib (symbol lib)
            lib (symbol (or (namespace lib) (name lib))
                        (name lib))
            alias (:alias opts)
            explicit-git-sha? (or (:sha opts) (:latest-sha opts))
            explicit-git-tag? (or (:tag opts) (:latest-tag opts))
            [version coord-type?]
            (cond explicit-git-tag?
                  [(or (and (:tag opts)
                            (git/find-github-tag lib (:tag opts)))
                       (git/latest-github-tag lib)) :git/tag]
                  ,,,)
            missing? (nil? version)
            mvn? (= coord-type? :mvn)
            git-sha? (= coord-type? :git/sha)
            git-tag? (= coord-type? :git/tag)
            git-url (when (or git-sha? git-tag?)
                      (or (:git/url opts)
                          (str "https://github.com/"
                               (git/clean-github-lib lib))))
            as (or (:as opts) lib)
            existing-aliases (-> edn-string
                                 edn/read-string
                                 :aliases)
            path (if alias
                   [:aliases
                    alias
                    (if (get-in existing-aliases [alias :deps])
                      :deps
                      :extra-deps)
                    as]
                   [:deps as])
            nl-path (if (and alias
                             (not (contains?
                                   existing-aliases
                                   alias)))
                      [:aliases alias]
                      path)
            edn-nodes (-> edn-nodes
                          (r/assoc-in nl-path nil)
                          str
                          r/parse-string)
            nodes (cond
                    missing? edn-nodes
                    mvn?
                    (r/assoc-in edn-nodes path
                                {:mvn/version version})
                    ,,,)
            nodes (if-let [root (and (or git-sha? git-tag?)
                                     (:deps/root opts))]
                    (-> nodes
                        (r/assoc-in (conj path :deps/root) root))
                    nodes)
            s (str (str/trim (str nodes)) "\n")]
        (when-not missing?
          (spit (:deps-file opts) s))))))

Source.

Do you notice a pattern? Consider scrolling back up to read. Forming a hypothesis for yourself before reading more of my opinions will help you reflect and remember!















Let’s continue.

The code is imperative! It uses the same variable names multiple times. Is deps a map from libraries to versions, or a map with a key :deps that has a value that is a map from libraries to versions? Depending on where in the code you’re reading, it’s both!

So, should it be refactored? Should it be split up into more pieces?

On the first glance, it’s easy to say “yes! The function is too long! It doesn’t use threading and other nice Clojure constructs!” I do not think this code should be split up. Why is that?

I believe that utility is contextual. The contextual utility for this code is that it should be easy to understand (utility) for someone who reads and changes the code or its surroundings (context).

And as someone reading it, I find this let block to be as clear as daylight. I read the code. The code does what it says it does. I can understand the order of the code—from top to bottom. The function is one thing. There’s no way to abuse the implementation details of the function, because those details are inside the function.

Here’s a piece of my own code:

(defn olorm-create [{:keys [opts]}]
  (when (or (:help opts) (:h opts))
    (println (str/trim "
Usage:

  $ olorm create [OPTION...]

Allowed options:

  --disable-git-commands  Disable all Git commands.
  --disable-git-magic     Alias for --disable-git-commands
  --dry-run               Suppress side effects, print instead
  --help                  Show this helptext.
  --no-git-commands       Alias for --disable-git-commands
  --no-git-magic          Alias for --disable-git-commands
"))
    (System/exit 0))
  (let [repo-path (repo-path)
        dispatch (fn [cmd & args]
                   (if (:dry-run opts)
                     (prn `(~cmd ~@args))
                     (apply (resolve cmd) args)))
        disable-git-commands (or (:disable-git-commands opts) ,,,)]
    (when-not disable-git-commands
      (dispatch `shell {:dir repo-path} "git pull --ff-only"))
    (let [number (inc (or (->> (olorm/docs {:repo-path repo-path})
                               (map :number) sort last)
                          0))
          doc (olorm/->olorm {:repo-path repo-path :number number})]
      (dispatch `fs/create-dirs (olorm/path doc))
      (let [index-md-path (olorm/index-md-path doc)]
        (dispatch `spit index-md-path (olorm/md-skeleton doc))
        (dispatch `spit (olorm/meta-path doc)
                  (prn-str {:git.user/email (olorm/git-user-email
                                             {:repo-path repo-path})
                            :doc/created (olorm/today)
                            :doc/uuid (olorm/uuid)}))
        (dispatch `shell {:dir repo-path}
                  (System/getenv "EDITOR") index-md-path)
        (when-not disable-git-commands
          (dispatch `shell {:dir repo-path} "git add .")
          (dispatch `shell {:dir repo-path} "git commit -m"
                    (str "olorm-" (:number doc)))
          (dispatch `shell {:dir repo-path} "git pull --rebase")
          (dispatch `shell {:dir repo-path} "git push")))
      (println
       (str
        "Husk å publisere i #mikrobloggeriet-announce på Slack. Feks:"
        "\n\n"
        "   OLORM-" (:number doc)
        ": $DIN_TITTEL → https://mikrobloggeriet.no/o/"
        (:slug doc) "/")))))

Source.

There are let expressions, and there is some code. Reading it now, I feel like I want to flatten it down to a single layer of let expressions. Something like this:

,,, #_ "stuff ..."
(let [repo-path (repo-path)
      dispatch (fn [cmd & args]
                 (if (:dry-run opts)
                   (prn `(~cmd ~@args))
                   (apply (resolve cmd) args)))
      disable-git-commands (or (:disable-git-commands opts) ,,,)
      _ (when-not disable-git-commands
          (dispatch `shell {:dir repo-path} "git pull --ff-only"))
      number (inc (or (->> (olorm/docs {:repo-path repo-path})
                           (map :number) sort last)
                      0))
      doc (olorm/->olorm {:repo-path repo-path :number number})
      _ (dispatch `fs/create-dirs (olorm/path doc))
      ,,, #_ "more stuff ..."])

But still, it’s fine. It’s one function. It doesn’t do anything fancy. It doesn’t need to do anything fancy! It’s just code, from top to bottom.

Sure, fancy code can sing. Abstractions can be beautiful. But there’s beauty in simplicity! So, go forth and write clear Clojure code. In many cases, a let is all you need. You don’t need fancy constructs to give your code clarity.

Do you agree? Disagree? Let me know!

Teodor