A crude clojure progress reporting function 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 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))))


Here's a testing function

(defn test-run-w-reporting [stop-clj-time]
  (def testing-func (fn [item] (. java.lang.Thread sleep (rand 1000))))
  (let [sequence-function-maps [{:sequence (range 30), :function testing-func},
                               {:sequence (range 15), :function testing-func}]]
    (run-with-reporting sequence-function-maps stop-clj-time)))
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. &amp; 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 &amp; :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. &amp; 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))))


Now, lets take a look at the function itself. In general, when
writing in clojure you should try to keep your functions generic
and simple. By keeping them simple and focusing on only a single
task, they are usually easier to compose and reuse.
I would thus recommend splitting up your function.

In your case what you want to do is really:


map a function over a sequence: clojure already provides a function for that, namely MAP.
stop the mapping operation if a specified time has been reached.
print out the average time per call
print out total time taken


The first point is trivial.

For the second point, we can exploit the fact that clojure has lazy
sequences. This allows us to use the built-in TAKE-WHILE function
with a predicate which checks the time and stops when it has passed
your stop-time:

(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 (&gt; (current-time) stop-time))
       true
       (do (println "stopped at " (current-time) ". Requested stop at " stop-time)
           false)))
   coll))


The third point can be achieved by simply mapping over a collection
and printing out the necessary information. This again is really
just what you did, but refactored into a function that works on any
sequence:

(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 &amp; {: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)))


Before writing the reporting function, a little helper so we can
get around seq-chunking in clojure:

(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))))))


Now we can write the actual reporting function:

(defn run-with-reporting [sequence-function-maps stop-time]
  (let [beginning-time (current-time) ;; 1.
        seq-counts (map (comp count :sequence) sequence-function-maps)
        total-sequence-counts (reduce + seq-counts)
        intermed-count (atom 0)]
    (println (str "total counts = " total-sequence-counts))
    ;; note how it is possible to immediately destructure the map
    (doseq [{:keys [sequence function]} sequence-function-maps]
      (swap! intermed-count
             +
             (count
              ;; this is where everything happens: apply function,
              ;; take-until stop-time reached &amp; measure average time
              (measure-coll-retrieval
               (take-until stop-time (map function (unchunk sequence)))
               :start-count @intermed-count
               :total-count total-sequence-counts))))
    (println "Total time taken = " (hrs-min-sec (- (current-time) beginning-time))
             ", Done = " @intermed-count
             "/" total-sequence-counts)))

(defn test-run-w-reporting [stop-clj-time]
  (def testing-func (fn [item] (. java.lang.Thread sleep (rand 1000))))
  (let [sequence-function-maps [{:sequence (range 30), :function testing-func},
                                {:sequence (range 15), :function testing-func}]]
    (run-with-reporting sequence-function-maps stop-clj-time)))


I hope this has helped you some.

Kind regards,
Thomas