biff 2022-10-12 | Slack Archive

Oh, haha… well, the following would throw an error after a short amount of time had passed unless I cider-connect’ed and evaluated the first form before that happened:

(ns wiki.treefrogwiki.db
  (:require [xtdb.api :as xt]
            [com.biffweb :as biff]))

(def node (:biff.xtdb/node @com.biffweb/system))

(def treefrog-data [{:treefrog/latin-name "Hyla cinerea"          :treefrog/common-name "Green Treefrog"    :treefrog/img "/img/Green_treefrog.jpeg"}
                    {:treefrog/latin-name "Agalychnis callidryas" :treefrog/common-name "Red-Eyed Treefrog" :treefrog/img "/img/red-eyed-tree-frog.jpg"}
                    {:treefrog/latin-name "Dryophytes gratiosus"  :treefrog/common-name "Barking Treefrog"  :treefrog/img "/img/barking-treefrog.png"}
                    {:treefrog/latin-name "Litoria caerulea"      :treefrog/common-name "White's Treefrog"  :treefrog/img "/img/whites-treefrog.png"}])

(defn add-data [docs] (for [item docs]
                        (as-> item thedoc
                          (merge thedoc {:xt/id (java.util.UUID/randomUUID)})
                          (xt/submit-tx node [[:xtdb.api/put thedoc]]))))

#_(add-data treefrog-data)

(defn full-query [] (xt/q (xt/db node)
                          '{:find [(pull e [*])]
                            :where [[e :xt/id id]]}))

#_(full-query)

and with the new version I don’t seem to need to manually evaluate the (db node… line. That said, I’m guessing I’m not going about this right (I was thinking of posting this code in this channel later anyway for a review).

Jacob O'Bryant05:10:23

hmm... i am surprised the new version would make a difference, but maybe there is some subtle thing happening with the order that things are getting evaluated in. but yeah, looks like not evidence of a regression. I have some feedback which I'll type up tomorrow when I'm at my desk!

macrobartfast05:10:04

All good! (the last thing I wanted to subject you to tonight after a sweet milestone like this is a code review)

🙂 1

Jacob O'Bryant18:10:30

So first thing is that as a rule of thumb, the only place I would ever use @com.biffweb/system directly is in the repl.clj file, or in comment/`#_ forms. (Since @com.biffweb/system` should only be accessed directly if you're doing stuff manually in the repl; in regular application code, it should always be taken as a parameter from somewhere else). e.g. in this case I'd rewrite add-data so it looks like (defn add-data [node docs] ...) , and then change the line below to be #_(add-data (:biff.xtdb/node @com.biffweb/system) treefrog-data). and do a similar thing with full-query (have it take db as a parameter). After those changes, you can delete the (def node ...) line. If this code is only used from the repl, I would move it into the repl.clj namespace. plus there's already the get-sys function there which makes things a little less verbose. Another advantage of using the get-sys function is that you're getting the value of the system atom right when you call it, so it's ~guaranteed to have been populated. I suspect the bug you got earlier was because the @com.biffweb/system was being evaluated before the system had finished starting, so the atom was empty (until you re-evaluated it later, after the system had started). The last thing is add-data again. Instead of for , you should use doseq:

(defn add-data [node docs]
  (doseq [item docs]
    (as-> item thedoc
      (merge thedoc {:xt/id (java.util.UUID/randomUUID)})
      (xt/submit-tx node [[:xtdb.api/put thedoc]]))))

for is lazy and should generally only be used with pure code. Since this is imperative/side-effectful (i.e. you're not having the for return a list of values, you just need it to call submit-tx ), doseq is the right thing to use. That being said, an even better refactoring would be to build up a single transaction with all the docs and then call submit-tx only once:

(defn add-data [node docs]
  (xt/submit-tx node
    (for [item docs]
      [:xtdb.api/put (merge item {:xt/id (java.util.UUID/randomUUID)})])))

In this case, for is the right choice because now its body is pure; you're having for return a list of transaction operations, and then only after you get the result you call submit-tx.

macrobartfast00:10:04

This is fantastic… thank you! I hadn’t even thought about repl.clj. I’ll start working through your review now. 😃

👌 1

2022-10-12

Channels