Excessive use of let a problem in Clojure?

I have this set of functions - it works, but I'm concerned about the use of lets.

(defn- create-counts [coll]
  "Computes how many times did each 'next state' come from a 'previous state'.
   The result type is {previous_state {next_state count}}."
  (let [past    coll
        present (rest coll)
        zipped  (map vector past present)
        sorted  (sort zipped)
        grouped (group-by first sorted)
        seconds (map (fn [[key pairs]] [key (map second pairs)]) (seq grouped))
        freqs   (map (fn [[key secs]]  [key (frequencies secs)]) seconds)]
    (into {} freqs)))

I started learning functional programming in Haskell some time ago, and there, because of laziness, it's commonplace to use this equivalent of Clojure let, because what's not needed, isn't ever computed:

asnwer = z
  where
    notused = map (^1000) [1000..10000]
    x = [1..]
    y = take 10 x
    z = map (*2) y

• Is it okay to use Clojure let in this way? Look for example at the create-counts fn definition - I use it quite heavily there.

• Would it be better to just put it all inside one expression?

• What's the best practice for this?

I like how everything's named, but I don't know if there's not a performance penalty hidden somewhere.

When I try to match the Haskell example with Clojure code, it seems there's eager evaluation:

(let [notused (do (Thread/sleep (rand 10000000)) :done)
      x       (iterate inc 1)
      y       (take 10 x)
      z       (map #(* 2 %) y)]
  z)

;=> sleeps till the judgement day

I could imagine preventing this with futures or some clever let-like macro, but really it seems to me that I'm trying to do something Haskell-y (lazy) in Clojure. What's the idiomatic way?

First of all, you could use abstract structural binding (a.k.a. destructuring) to reduce the number of let bindings:

(defn- create-counts_org [[_ & present :as coll]]
  "Computes how many times did each 'next state' come from a 'previous state'.
   The result type is {previous_state {next_state count}}."
  (let [zipped  (map vector coll present)
        sorted  (sort zipped)
        grouped (group-by first sorted)
        seconds (map (fn [[key pairs]] [key (map second pairs)]) (seq grouped))
        freqs   (map (fn [[key secs]]  [key (frequencies secs)]) seconds)]
    (into {} freqs)))

I think there's nothing wrong in using let bindings that are technically not necessary to improve readability and comprehensibility.

But in your function, I think it is pretty clear what every step does:

• sorted -> the first thing I see is the sort function

• grouped -> the first thing I see is the group-by function

• seconds / freqs -> I think these are also pretty clear, using second and frequencies

And because each let binding just uses the previous binding once, I would probably use the thread-last (->>) macro:

(defn- create-counts [[_ & rest :as coll]]
  "Computes how many times did each 'next state' come from a 'previous state'.
   The result type is {previous_state {next_state count}}."
  (->>  (map vector coll rest)
        (sort)
        (group-by first)
        (map (fn [[key pairs]] [key (map second pairs)]))
        (map (fn [[key secs]]  [key (frequencies secs)]))
        (into {})))

I think this also answers your better to just put it all inside one expression question. Also, even if there where a performance penalty in using let bindings, it would probably be negligible.

StackExchange Code Review Q#22934, answer score: 5