I certainly prefer giving names to intermediate results with a "let" block: having good names and breaking the computation up into logical chunks makes the code much easier to understand and maintain when you come back to it later.
PG's example though is bad for different reasons - this is actually mutating variables in an imperative style, which is definitely "bad style" - both in Lisp and Clojure I think. The Clojure equivalent would be to use atoms (or vars) and mutating them. "let" on its own is purely functional, and doesn't have this problem. On Tuesday, 15 October 2013 20:29:29 UTC+8, Daniel Higginbotham wrote: > > I've been going through On Lisp by Paul Graham and on page 33 he > recommends against performing "intermediate" bindings. Does this advice > hold for Clojure? Here are a couple examples: > > ;; Common Lisp (from the book) > (defun bad (x) > (let (y sqr) > (setq y (car x)) > (setq sqr (expt y 2)) > (list 'a sqr))) > > (defun good (x) > (list 'a (expt (car x) 2))) > > ;; Clojure > (defn bad [x] > (let [y (first x) > sqr (expt y 2)] > (list 'a sqr))) > > (defn good [x] > (list 'a (expt (first x) 2))) > > Paul Graham explains: > > "The final result is shorter than what we began with, and easier to > understand. In the original code, we’re faced with the final expression > (list 'a sqr), and it’s not immediately clear where the value of sqr comes > from. Now the source of the return value is laid out for us like a road > map. > > The example in this section was a short one, but the technique scales up. > Indeed, it becomes more valuable as it is applied to larger functions." > > In clojure you can't do setq of course but I find myself going against > this advice all the time, and I find that it's more important to do so when > working with larger functions. I think introducing names makes code > clearer. Here's an example from my own code: > > (defn create-topic > [params] > (let [params (merge params (db/tempids :topic-id :post-id :watch-id)) > topic (remove-nils-from-map (c/mapify params mr/topic->txdata)) > watch (c/mapify params mr/watch->txdata) > post (c/mapify params mr/post->txdata)] > {:result (db/t [topic post watch]) > :tempid (:topic-id params)})) > > To my mind, creating bindings for "topic", "watch", and "post" makes the > code easier to understand. When you get to "(db/t [topic post watch])" you > don't have to deal with as much visual noise to understand exactly what's > going into the transaction. > > So, is PG's advice any good? > > Thanks! > Daniel -- -- You received this message because you are subscribed to the Google Groups "Clojure" group. To post to this group, send email to clojure@googlegroups.com Note that posts from new members are moderated - please be patient with your first post. To unsubscribe from this group, send email to clojure+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/clojure?hl=en --- You received this message because you are subscribed to the Google Groups "Clojure" group. To unsubscribe from this group and stop receiving emails from it, send an email to clojure+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.