patternMinor

A crude clojure progress reporting function

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

reportingclojurecrudefunctionprogress

Problem

The purpose of this function is to report processing progress to the terminal. It loops through an array of maps that contain two properties: :sequence and :function. Within each sequence-function-map, it will loop through the sequence, and run the function on each item in the sequence.

This is my first Clojure program that will be used in a production environment. This particular function does the job, but I suspect that it's not very lispy.

```
(defn run-with-reporting [sequence-function-maps stop-time]
(def beginning-time (java.util.Date.))
(let [seq-counts (map #(count (:sequence %)) sequence-function-maps)]
(def total-sequence-counts (reduce + seq-counts)))
(println (str "total counts = " total-sequence-counts))
(def prog-rpt-ref (ref {:todo total-sequence-counts, :done 0, :total-time-taken 0}))
(doseq [sequence-function-map sequence-function-maps]
(let [sequence (:sequence sequence-function-map),
function (:function sequence-function-map)]
(loop [loop-seq sequence]

(if (and stop-time (clj-time/after? (clj-time/now) stop-time))
(println "stopped at " (clj-time/now) ". Requested stop at " stop-time)
(if loop-seq
(do
(def item (first loop-seq))
(def start-time (java.util.Date.))
(function item)
(def end-time (java.util.Date.))
(def time-delta (- (.getTime end-time) (.getTime start-time)))
(let [derefd-rpt (deref prog-rpt-ref)]
(dosync (alter prog-rpt-ref assoc
:done (inc (:done derefd-rpt)),
:total-time-taken (+ (:total-time-taken derefd-rpt) time-delta))))
(let [derefd-rpt (deref prog-rpt-ref)]
(let [average-time (/ (:total-time-taken derefd-rpt) (:done derefd-rpt))]
(println "Avg time / each = " (hrs-min-sec average-time)
", Estimated total tim

Solution

I will point out stuff that I'd do differently, though I am by no means a clojure expert, I do have some experience in it. Numbered items will reappear inside the code as comments.

no DEF inside defn body

def will define the var for your entire namespace as such, you will pollude your namespace with temporaries

use LET instead

no DO necessary inside the body of IF

furthermore, I find it more idiomatic to use WHEN in the case that no else branch exists

using @ instead of DEREF could reduce verbosity

by using UPDATE-IN instead of INC, you can simply pass a transformation function

note the vector notation however: UPDATE-IN takes a vector of keywords

You can pass a partial function, i.e. a function which is

partially applied as the transformation function to update the total-time-taken

No need to nest LETs, you can write all definitions into a single let

Code:

(defn run-with-reporting [sequence-function-maps stop-time]
  (let [beginning-time (java.util.Date.) ;; 1.
        seq-counts (map #(count (:sequence %)) sequence-function-maps)
        total-sequence-counts (reduce + seq-counts)
        prog-rpt-ref (ref {:todo total-sequence-counts, :done 0, :total-time-taken 0})]
    (println (str "total counts = " total-sequence-counts))
    (doseq [sequence-function-map sequence-function-maps]
      (let [sequence (:sequence sequence-function-map),
            function (:function sequence-function-map)]
        (loop [loop-seq sequence]
          (if (and stop-time (clj-time/after? (clj-time/now) stop-time))
            (println "stopped at " (clj-time/now) ". Requested stop at " stop-time)
            (when loop-seq ;; 2.
              (let [item (first loop-seq)
                    start-time (java.util.Date.)]
                (function item)
                (let [end-time (java.util.Date.)
                      time-delta (- (.getTime end-time) (.getTime start-time))
                      derefd-rpt @prog-rpt-ref]         ;; 3.
                  (dosync (alter prog-rpt-ref update-in ;; 4.
                                 [:done] inc,
                                 [:total-time-taken] (partial + time-delta))) ;; 5.
                  (let [derefd-rpt @prog-rpt-ref ;; 3. & 6.
                        average-time (/ (:total-time-taken derefd-rpt) (:done derefd-rpt))]
                    (println "Avg time / each = " (hrs-min-sec average-time)
                             ", Estimated total time left = "
                             (hrs-min-sec (* average-time (- (:todo derefd-rpt) (:done derefd-rpt)))))))
                (recur (next loop-seq))))))))
    (let [derefd-rpt (deref prog-rpt-ref)]
      (println "Total time taken = " (hrs-min-sec (- (.getTime (java.util.Date.)) (.getTime beginning-time))) ", Done = " (:done derefd-rpt) "/" (:todo derefd-rpt))))
  )

-
I would try to reduce verbosity and increase readability (in the
literate sense) by utilising descriptive helper functions -
current-time instead of (.getTime (java.util.Date.))

-
Personally I like to keep the amount of variables as low as
possible. Variables are great for producing side-effects,
describing complex results or for when you need a result several
times. In the case of the ref, writing @ instead of using a
temporary seems more natural to me.

As such:

I would try to reduce the amount of side-effects and

use descriptive functions to calculate complex results and place them where they belong

without temporary variables

You can use destructring for maps

(let [{:keys [key1 key2]} mymap] ...) will let-bind key1 and key2 by extracting :key1 & :key2 from mymap

Code:

```
(defn current-time
"Return the current time. Same as (.getTime (java.util.Date.))."
[]
(.getTime (java.util.Date.)))

(defn run-with-reporting [sequence-function-maps stop-time]
(let [beginning-time (current-time) ;; 1.
seq-counts (map #(count (:sequence %)) sequence-function-maps)
total-sequence-counts (reduce + seq-counts)
prog-rpt-ref (ref {:todo total-sequence-counts, :done 0, :total-time-taken 0})]
(println (str "total counts = " total-sequence-counts))
(doseq [sequence-function-map sequence-function-maps]
(let [{:keys [sequence function]} sequence-function-map] ;; 9.
(loop [loop-seq sequence]
(if (and stop-time (clj-time/after? (clj-time/now) stop-time))
(println "stopped at " (clj-time/now) ". Requested stop at " stop-time)
(when loop-seq ;; 2.
(let [item (first loop-seq)
start-time (current-time)] ;; 7.
(function item)
(dosync (alter prog-rpt-ref update-in ;; 4.
[:done] inc,
[:total-time-taken] (partial + (- (current-time) start-time)))) ;; 5., 7. & 8.
(let [average-time (/ (:total-time-taken @prog-rpt-ref) (:done @prog-rpt-ref))] ;; 8.
(println "Avg time / each = " (hrs-mi

Code Snippets

(defn run-with-reporting [sequence-function-maps stop-time]
  (let [beginning-time (java.util.Date.) ;; 1.
        seq-counts (map #(count (:sequence %)) sequence-function-maps)
        total-sequence-counts (reduce + seq-counts)
        prog-rpt-ref (ref {:todo total-sequence-counts, :done 0, :total-time-taken 0})]
    (println (str "total counts = " total-sequence-counts))
    (doseq [sequence-function-map sequence-function-maps]
      (let [sequence (:sequence sequence-function-map),
            function (:function sequence-function-map)]
        (loop [loop-seq sequence]
          (if (and stop-time (clj-time/after? (clj-time/now) stop-time))
            (println "stopped at " (clj-time/now) ". Requested stop at " stop-time)
            (when loop-seq ;; 2.
              (let [item (first loop-seq)
                    start-time (java.util.Date.)]
                (function item)
                (let [end-time (java.util.Date.)
                      time-delta (- (.getTime end-time) (.getTime start-time))
                      derefd-rpt @prog-rpt-ref]         ;; 3.
                  (dosync (alter prog-rpt-ref update-in ;; 4.
                                 [:done] inc,
                                 [:total-time-taken] (partial + time-delta))) ;; 5.
                  (let [derefd-rpt @prog-rpt-ref ;; 3. & 6.
                        average-time (/ (:total-time-taken derefd-rpt) (:done derefd-rpt))]
                    (println "Avg time / each = " (hrs-min-sec average-time)
                             ", Estimated total time left = "
                             (hrs-min-sec (* average-time (- (:todo derefd-rpt) (:done derefd-rpt)))))))
                (recur (next loop-seq))))))))
    (let [derefd-rpt (deref prog-rpt-ref)]
      (println "Total time taken = " (hrs-min-sec (- (.getTime (java.util.Date.)) (.getTime beginning-time))) ", Done = " (:done derefd-rpt) "/" (:todo derefd-rpt))))
  )

(defn current-time
  "Return the current time. Same as (.getTime (java.util.Date.))."
  []
  (.getTime (java.util.Date.)))

(defn run-with-reporting [sequence-function-maps stop-time]
  (let [beginning-time (current-time) ;; 1.
        seq-counts (map #(count (:sequence %)) sequence-function-maps)
        total-sequence-counts (reduce + seq-counts)
        prog-rpt-ref (ref {:todo total-sequence-counts, :done 0, :total-time-taken 0})]
    (println (str "total counts = " total-sequence-counts))
    (doseq [sequence-function-map sequence-function-maps]
      (let [{:keys [sequence function]} sequence-function-map] ;; 9.
        (loop [loop-seq sequence]
          (if (and stop-time (clj-time/after? (clj-time/now) stop-time))
            (println "stopped at " (clj-time/now) ". Requested stop at " stop-time)
            (when loop-seq ;; 2.
              (let [item (first loop-seq)
                    start-time (current-time)] ;; 7.
                (function item)
                (dosync (alter prog-rpt-ref update-in ;; 4.
                               [:done] inc,
                               [:total-time-taken] (partial + (- (current-time) start-time)))) ;; 5., 7. & 8.
                (let [average-time (/ (:total-time-taken @prog-rpt-ref) (:done @prog-rpt-ref))] ;; 8.
                  (println "Avg time / each = " (hrs-min-sec average-time)
                           ", Estimated total time left = "
                           (hrs-min-sec (* average-time (- (:todo @prog-rpt-ref) (:done @prog-rpt-ref)))))) ;; 8.
                (recur (next loop-seq))))))))
    (println "Total time taken = " (hrs-min-sec (- (current-time) beginning-time))
             ", Done = " (:done @prog-rpt-ref)
             "/" (:todo @prog-rpt-ref))))

(defn take-until
  "Returns a lazy seq of items from coll util the STOP-TIME has been reached."
  [stop-time coll]
  (take-while
   (fn [item]
     (if (not (> (current-time) stop-time))
       true
       (do (println "stopped at " (current-time) ". Requested stop at " stop-time)
           false)))
   coll))

(defn measure-coll-retrieval
  "Returns a lazy seq of items from coll. Will print to stdout the
  average time between element extractions."
  ;; you can use destructuring inside the argument list as well, here
  ;; we're additionaly specifying some defaults for when the caller
  ;; does not provide a value
  [coll & {:keys [start-count total-count] :or {start-count 0 total-count nil}}]
  (let [beginning-time (current-time)]
    (map-indexed
     (fn [index item]
       (let [index (+ start-count index 1)
             average-time (/ (- (current-time) beginning-time) index)]
         (print "Avg time / each = " (hrs-min-sec average-time))
         (if total-count
           (println ", Estimated total time left = "
                    (hrs-min-sec (* average-time (- total-count index))))
           (println))
         item))
     coll)))

(defn unchunk
  "takes a chunked sequence and turns it into an unchunked sequence"
  [s]
  (lazy-seq
   (when-let [[x] (seq s)]
     (cons x (unchunk (rest s))))))

Context

StackExchange Code Review Q#4956, answer score: 5

Revisions (0)

No revisions yet.