← Back to context

Comment by geokon

3 days ago

I find 95% Clojure has the right tools to write very terse code. But in some cases the functional transducer/piped paradigm can't be contorted to the problem.

Usually these are problems where you need to run along a list and check neighboring elements. You can use amap or map-indexed but it's just not ergonomic or Clojure-y (vs for instance the imperative C++ iterator model)

The best short example I can think of is Fibbonacci

https://4clojure.oxal.org/#/problem/26/solutions

I find all the solutions hard to read. They're all ugly. Their performance characteristics are hard to know at a glance

Personally, I would normally reach for loop to check neighboring elements very ergonomically.

  (loop [[a b c & more] coll] (recur (apply list b c more)))

There’s also partition if you're working with transducers/threads/list comprehension

  (partition 3 1 coll)

Or if you need to apply more complicated transformations to the neighbors/cycle the neighbors

  (->> coll cycle rest (map xform) (map f coll))

Using map-indexed to look up related indices is something I don’t think I do anywhere in my codebase. Agreed that it’s not ergonomic

EDIT: those Fibonacci functions are insane, even I don’t understand most of them. They’re far from the Clojure I would advocate for, most likely written for funsies with a very specific technical constraint in mind

  • Yeah, I guess partition to me always looks like a dangerous tool - for instance I have a sequence of numbers and I want to do a 5 point rolling average

    You could do `(partition 5 1 coll)` and then average each element in the resulting seq.. It's very easy to reason about. But I'm guessing the performance will be abysmal? You're getting a lazy list and each time you access a 5 neighbor set.. you're rerunning down you coll building the 5 unit subsets? Maybe if you start with an Array type it'll be okay, but you're always coercing to seq and to me it's hard

    Taking the first 5 elements, recurring on a list with the top element dropped is probably better, but I find the code hard to read. Maybe it's a familiarity issue..

    • Yeah like I said I reach for loop first and foremost. This is what it would look like with comments if it were actually something complicated (although the comments are quite trivial here):

        (loop [coll [1 2 3 4 5 6 7 8 9] memo []]
          (if (< (count coll) 5) memo ;; Stop once there are less than 5 items
            (->> (take 5 coll)        ;; Take the first 5 days
                 (reduce +)           ;; Sum them
                 (* 0.20)             ;; Divide by 5
                 (conj memo)          ;; Append to the end of the memo
                 (recur (rest coll))  ;; Recur for remaining elements
                 ,,,))))
      

      Realistically if performance was a consideration I would probably do:

        (loop [[a b c d e :as coll] [1 2 3 4 5 6 7 8 9] memo []]
          (if-not e memo
            (recur (rest coll) (conj memo (/ (+ a b c d e) 5))))))
      

      Should be ~15 times faster to avoid the nested loop. If you want to change the min size it's still pretty clean:

        (loop [[a b c d e :as coll] [1 2 3 4 5 6 7 8 9] memo []]
          (if-not c memo
            (recur (rest coll) (conj memo (/ (+ a b c d e) 
                                             (cond e 5 d 4 c 3))))))))

      6 replies →

  • Exactly. I would use loop or partition+map/reduce for that case. I almost never use map-indexed. In fact I almost never use indexing at all. Mostly, when I have a sequential collection (vector, list, or generic seq), I need to iterate over all the elements, and I’m doing that with map or reduce. IMO, map-indexed has a code-smell that indicates that you’re reaching for an imperative algorithm when perhaps a functional algorithm would be better. Surely, there are times when map-indexed is just what you need, which is why it’s there, but typically not in my experience.

    • I would agree that map-indexed + *nth* is a code smell. I use map-indexed all the time though, just when I need the index, not other elements in the list