Getting rid of extra test during initialization of loop/recursion

Question

I'm reluctant to ask this question. My code below works, it's intelligible, and it seems reasonably efficient. It's just that there's a trivial, nitpicky issue that's driving me crazy. The function maxes below collects all elements of a sequence that are "maximum" according to some criterion.

;; Example of use of maxes
;; Collect all maps with the maximum value for `:a`:
(maxes :a [{:a 1 :b 2} {:a 4 :b 5} {:a 5 :b 5} {:b 3 :a 5}])
;;=> [{:b 5, :a 5} {:b 3, :a 5}]

(defn- maxes-helper
  "Helper function for maxes."
  [f s best-val collected]
  (if (empty? s)
    collected
    (let [new-elt (first s)
          new-val (f new-elt)]
      (cond (== new-val best-val) (recur f (rest s) best-val (conj collected new-elt))
            (>  new-val best-val) (recur f (rest s) new-val  [new-elt])
            :else                 (recur f (rest s) best-val collected)))))

(defn maxes
  "Returns a sequence of elements from s, each with the maximum value of (f element)."
  [f s]
  (if (empty? s)
    nil
    (let [new-elt (first s)
          new-val (f new-elt)]
      (maxes-helper f (rest s) new-val [new-elt]))))

What's really bugging me that I have to test for emptiness of the input collection s both in the top-level function and in the tail-recursive helper function. I also want to add a custom exception when (f new-elt) is non-numeric, and so I need to test that twice, both for new-val and for first call to (f new-elt) that becomes the first best. So I've got a trivial amount of duplication of code, and I keep thinking that there must be a way to get rid of the duplication. But I can't see how to do this (without testing that best is numeric on every iteration). Am I missing something obvious (or non-obvious)?

(BTW I wrote another version using reduce, but it's about twice as slow, according to Criterium.)

Answer 1

You don't really need the helper function, the original maxesis pretty much equivalent to call maxes-helper with a sensible MIN_VALUE and empty sequence, so I refactored and joined both for you leveraging multi-arity

   (defn maxes
      "Returns a sequence of elements from s, each with the maximum value of (f element)."
      ([f s]
       (maxes f s Double/NEGATIVE_INFINITY '()))
      ([f s best-val collected]
      (if (empty? s)
        collected
        (let [new-elt (first s)
              new-val (f new-elt)]
          (cond (>  new-val best-val) (recur f (rest s) new-val  [new-elt])
                (== new-val best-val) (recur f (rest s) best-val (conj collected new-elt))
                :else                 (recur f (rest s) best-val collected))))))

Edited to replace nil by Double/NEGATIVE_INFINITY as suggested by @fmikes