This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
I could use help cleaning up a clojure function I was doing for advent of code. I find nested if checks with different calls to recur
really hard to read. Is there a way to make this more readable without extracting parts of it to other functions? One special consideration is the array we loop through may dynamically change over time, so using for
doesn't really work.
(defn process-intervals [seed-intervals]
(loop [intervals seed-intervals
min-number ##Inf]
(if-let [[current-interval & remaining-intervals] (seq intervals)]
(let [[start _ current-interval-rules] current-interval]
(if (empty? current-interval-rules)
(recur remaining-intervals (min min-number start))
(let [result-of-interval-process
(process-interval-through-rules-row current-interval)]
(if result-of-interval-process
(recur
(concat result-of-interval-process remaining-intervals) min-number)
(recur remaining-intervals min-number)))))
min-number)))
Here's a rewrite, done without thinking about the algorithm at all so maybe there's room for improvement:
(defn process-intervals [seed-intervals]
(loop [intervals seed-intervals
min-number ##Inf]
(if-let [[current-interval & remaining-intervals] (seq intervals)]
(let [[start _ current-interval-rules] current-interval]
(if (empty? current-interval-rules)
(recur remaining-intervals (min min-number start))
(let [intervals (concat (process-interval-through-rules-row current-interval)
remaining-intervals)]
(recur intervals min-number))))
min-number)))
Thinking about the algorithm itself a bit more, this is how I'd write it:
(defn process-intervals [seed-intervals]
(letfn [(has-rules? [[_start _ rules]]
(seq rules))]
(->> (tree-seq has-rules?
process-interval-through-rules-row
seed-intervals)
(remove has-rules?)
(map first)
(reduce min ##Inf))))
Nothing, but you can't extract much due to the nested recur
calls, so you would mostly be extracting tiny things that probably would make it less readable.
@U2FRKM4TW I'm checking out your second answer. It crashes for me because of what the functions are doing, but it's got fresh stuff I haven't seen. Maybe I just need to change one thing. Will get back to you.
The first suggestions helped. I didn't realize (concat nil [1 2])
was the same as [1 2]
I think tree-seq
might not be appropriate. The rules are an array of rows of rules. Like this
[[{:source-range-start 98, :source-range-end 100, :delta -48}
{:source-range-start 50, :source-range-end 98, :delta 2}]
[{:source-range-start 15, :source-range-end 52, :delta -15}
{:source-range-start 52, :source-range-end 54, :delta -15}
{:source-range-start 0, :source-range-end 15, :delta 39}]
[{:source-range-start 53, :source-range-end 61, :delta -4}
{:source-range-start 11, :source-range-end 53, :delta -11}
{:source-range-start 0, :source-range-end 7, :delta 42}
{:source-range-start 7, :source-range-end 11, :delta 50}]
[{:source-range-start 18, :source-range-end 25, :delta 70}
{:source-range-start 25, :source-range-end 95, :delta -7}]
[{:source-range-start 77, :source-range-end 100, :delta -32}
{:source-range-start 45, :source-range-end 64, :delta 36}
{:source-range-start 64, :source-range-end 77, :delta 4}]
[{:source-range-start 69, :source-range-end 70, :delta -69}
{:source-range-start 0, :source-range-end 69, :delta 1}]
[{:source-range-start 56, :source-range-end 93, :delta 4}
{:source-range-start 93, :source-range-end 97, :delta -37}]]
This is my first time seeing it though, I'm playing with it
I was wrong, I think this is working
BTW if the data is always a vector of vectors of maps, then there might be other solutions. Your initial solution and the tree-seq
solution both can work on an arbitrarily nested data, so I made no assumptions.
Something like this
(defn process-intervals [seed-intervals]
(letfn [(has-rules? [[_start _ rules]]
(seq rules))]
(reduce min ##Inf
(for [interval seed-intervals]
(->> (tree-seq has-rules?
process-interval-through-rules-row
interval)
(remove has-rules?)
(map first)
(reduce min ##Inf))))))
I am having a hard time grokking tree-seq
but that works
Ahh, of course, my bad. A slightly rearranged alternative:
(defn process-intervals [seed-intervals]
(letfn [(has-rules? [[_start _ rules]]
(seq rules))]
(->> seed-intervals
(mapcat (fn [interval]
(->> (tree-seq has-rules?
process-interval-through-rules-row
interval)
(remove has-rules?)
(map first))))
(reduce min ##Inf))))
That works too. Playing around with tree-seq some more and trying to wrap around how that maps to what I was doing
Thank you!
Your intervals are nested, so they form a tree. tree-seq
returns the nodes of that tree as a seq.
Hmm. The intervals are nested as in once the rules are applied to them? Because the array of rules doesn't seem nested
So it looks like if you want to apply a transformation to something, and it causes more of that thing in similar shapes that need to be processed the same way, tree-seq
is good for that.
And the same function that determines when to stop processing can be used to remove
the intermediate results.
I wrote some notes on tree-seq
this morning for myself. Thank you for introducing me to it.
Let me know if you have any ideas for this one too
(defn process-interval-through-rule [interval processing-rule]
;; Will take an interval and either return nil if there's no overlap
;; with the rule, or it will return an array of new intervals.
;; The new intervals will be where the ranges don't overlap, and
;; also where the interval does overlap with one row of rules GONE
;; This is similar to advancing the level of the rule.
;;
;; It doesn't care what is in interval-rules, it will merely remove the first
;; thing. It relies on processing-rule for the actual check.
(let [[interval-start interval-end interval-rules] interval
{:keys [source-range-start source-range-end delta]} processing-rule]
(if (or (<= interval-end source-range-start)
(<= source-range-end interval-start))
nil
(let [within-start (< interval-start source-range-start)
within-end (< source-range-end interval-end)]
(keep identity
[(when within-start
[interval-start source-range-start interval-rules])
(when within-end
[source-range-end interval-end interval-rules])
[(+ (if within-start source-range-start interval-start) delta)
(+ (if within-end source-range-end interval-end) delta)
(rest interval-rules)]])))))
Nothing groundbreaking, just some cosmetic stuff because I have different preferences (no single-branch if
s, ?
at the end of most boolean variables, (remove nil? ...)
instead of (keep identity ...)
).
If the order of the resulting array doesn't matter, I'd use cond->
with conj
to add items to a vector rather than remove them.
Do I have a single branch if?
Oh, almost forgot one tiny thing - you're using the word "array" where a correct term should be "vector". Clojure has both (arrays are just Java arrays), they're different.
Indeed, my bad
I'm trying to use the new clojure.repl.deps/add-lib
described in the https://clojure.org/news/2023/04/14/clojure-1-12-alpha2#_add_libraries_for_interactive_use, but as far as I can tell it requires 1.12 installed globally. Is there an ergonomic way of installing the 1.12 alpha globally? I tried fiddling around with the global deps.edn but that didn't seem to work. https://github.com/clojure/homebrew-tools talks about having pre-release versions, but it doesn't seem to have the 1.12 alpha.
> it requires 1.12 installed globally No. You don't install Clojure globally, you install only the CLI globally. And that feature requires the latest version of the CLI, not of Clojure itself.
E.g. the feature works just fine with 1.11.1.1435
.
But you have to run clj
with -Sdeps '{:deps {org.clojure/clojure {:mvn/version "1.12.0-alpha8"}}}'
in order for it to work.
uhm when I tried with alpha 2 locally, together with latest stable cli I got an error that I thought I traced down to missing functionality in the global cli.... but maybe I messed up along the way
gonna retry it right now
yeap you're absolutely right
[I] filipesilva@Filipes-MacBook-Pro ~/s/scratch (master) [SIGINT]> clj -Sdeps '{:deps {org.clojure/clojure {:mvn/version "1.12.0-alpha8"}}}'
Clojure 1.12.0-alpha8
user=> (require '[clojure.repl.deps :as deps])
nil
user=> (deps/add-lib 'org.clojure/data.csv {:mvn/version "1.1.0"})
[org.clojure/data.csv]
user=>
dunno what I did before that was broken, thanks for setting me on the right track!