Thanks Meikel for such an insightfull feedback. Its hard to imagine so much thought goes into writing trivial looking functions. Sunil.
On Thu, Nov 25, 2010 at 1:00 PM, Meikel Brandmeyer <m...@kotka.de> wrote: > Hi, > > On 25 Nov., 05:06, Sunil S Nandihalli <sunil.nandiha...@gmail.com> > wrote: > > > (defn every-nth [n coll] > > (letfn [(evn [cn s] > > (when s > > (if (= cn 1) > > (lazy-seq (cons (first s) (evn n (next s)))) > > (evn (dec cn) (next s)))))] > > (evn n coll))) > > Since you want to learn how lazy-seq works here some feedback. > > * Make the lazy-seq the outer-most part of your function > to allow as much laziness as possible. There other > opinions out there about the placement of lazy-seq, but > there are also a lot of people complaining about eg. a > filter doing an expensive predicate call because the > input sequence is not as lazy as it could be. By placing > lazy-seq outermost you leave the decision to them when > to realize an element. > > * You should call seq on coll before passing it to evn. > Think of [] as input. It will be truthy in the when check > and we do some unnecessary loop, where we actually could > short circuit. > > * Don't call next in the true branch of the if. It realises > an element of the input sequence where it is not > necessary. Use rest. > > * Don't call evn in the false branch of the if. It will be > true recursion and might blow the stack for large n. You > Use recur. > > Here is the version I would write in this case. > > (defn every-nth > [n coll] > (let [step (fn step [s] > (lazy-seq > (loop [s (seq s) > cnt n] > (when s > (if (= cnt 1) > (cons (first s) (step (rest s))) > (recur (next s) (dec cnt)))))))] > (step coll))) > > * lazy-seq is outer-most. > * The recur is turned into a loop to avoid stacking lazy-seq > on lazy-seq. > * The input sequence is only realised inside the lazy-seq. > Also in the true branch we use rest to defer realisation > of the next seq step. In the false branch we use next, > because we need the value anyway. > > As a rule of thumb: write your generating function with > "normal" recursion first. Then simply wrap a lazy-seq > around it. > > Hope that helps. > > Sincerely > Meikel > > -- > 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<clojure%2bunsubscr...@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 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
