Add assoc-some util method. Or use cond->.

I actually do have an assoc-if method I wrote long ago, but I wanted to check here if there was something more idiomatic. I didn’t think about cond->, looking….

https://ask.clojure.org/index.php/8387/how-to-avoid-nil-values-in-maps

nitpick: don't mix anonymous fns with threading macros

e.g. (->> coll (#(....)) is not idiomatic

I’ve heard that before. I just don’t find it less readable than fn, but I know a lot of people do.

also avoid changing between collection ops and scalar ops in a ->> macro

it's easy to miss the call to first on the second line of the thread

can you suggest how you’d split this out?

add more bindings to the let

ah ok

it's not a goal to coagulate everything into a ->>, it has some downsides

one downside is that forms are not named, so you gotta read all the recipe

also don't reach into a global atom inside your fns

pass arguments, it's testable

(defn calibration-update [cali-idx & {:keys [k-value n-coef]}]
  (let [[extrusion] (->> (:extrusion-cali @state)
                         (filter #(= (:cali_idx %) cali-idx)))]
    (cond-> extrusion
      k-value (assoc :k_value k-value)
      n-coef (assoc :n_coef n-coef))))

but I recommend passing the value of @state explicitly to this fn

(-> @state 
    :extrusion-cali
    (->> (filter ..))
    first
    (cond-> 
      k-value (assoc :k_value k-value)
      n-coef  (assoc :n_coef  n-coef)))

How about this?:

(defn calibration-update [cali-idx & {:keys [k-value n-coef]}]
  (let [calibrations (:extrusion-cali @state)
        target-calibration (first (filter #(= (:cali_idx %) cali-idx)
                                    calibrations))
        target-calibration-updated (cond-> target-calibration
                                     k-value (assoc :k_value k-value)
                                     n-coef (assoc :n_coef n-coef))]
    target-calibration-updated
    ;; Do stuff with cali
  ))

that's better

(and def. do not put multiple calls on a single line -- I totally missed first until I saw Ghadi's destructuring and I only barely noticed :extrusion-cali)

@U04V70XH6 you mean when threading yeah?

Yup. Either all on one line or all vertical -- don't mix them.

Ok, I like where this ended up, I’m glad I asked 👍. Thanks guys, learned some new stuff.

@U050ECB92 “also don’t reach into a global atom inside your fns” Can you elaborate on this a little? It seems like ultimately I’m going to have to access it from a function if I want the current state?

Pass the state as an argument, don't use a global, is the general advice.

I’m confused there - because then function that is calling this function, is then accessing this atom from inside a function.

(e.g. no matter what, I’m accessing this atom from inside a function)

You would need to pass the "system state" through your entire call chain from -main essentially.

Oh I see.

Makes it easier to test everything.

Just limit where we’re doing it.

And makes it clear that the functions depend on state.

In this case it’s a little trickier because the state is constantly being updated by another thread, but I understand what you are saying.

Pass the atom around, in general, and deref it in arguments to functions that only read its value perhaps?

Yeah ok - more food for thought there, thank you. I will review.

That also removes the possibility of a function that does @state in two places getting two different values -- except where you specifically need that to happen.

Yeah makes sense

(including two places in a single call chain -- so if f does @state and calls g which also does @state and they end up operating on two different states... that sort of thing would be very hard to debug)

Yeah I could see that. I don’t have control over when it’s updating - but I can make sure an entire given call chain is operating on a single fixed value. In this specific case, this function is the entire call chain (well, it has a cli-parent function that invokes it), but I see the potential problem.