# 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?

-
Just a note that the Haskell equivalent of the clojure let is let. where is a different beast –  jozefg Feb 25 '13 at 0:36
I'm a clojure noob, but the documentation for map says it returns a lazy sequence. Group-by is not lazy. clojuredocs.org/clojure_core/clojure.core/map –  GlenPeterson Feb 25 '13 at 13:57

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.

-
Thanks! I've seen the ->> macro before, but didn't think about it. Seems handy. –  Martin Janiczek Feb 28 '13 at 9:48