Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have been solving a set of online Clojure problems. One of them involves the look and say sequence.

(defn look-and-say [s]
   (let [las (loop [item (first s) coll s counts [] count 0]
      (cond
         (nil? (seq coll)) (conj counts count item)
         (= item (first coll)) (recur item (rest coll) counts (inc count))
         :else  (recur (first coll)  coll (conj counts count item) 0)))]
      (lazy-seq (cons las (look-and-say las)))))

I would like know what I did right and what I could do better. Any feedback is welcome.

share|improve this question

migrated from stackoverflow.com Sep 4 '11 at 3:14

This question came from our site for professional and enthusiast programmers.

up vote 3 down vote accepted

one of the great things about Clojure is the ability to seperate code into concerns and then compose them. This really helps spot reusable parts and lets you build code in smaller independently testable parts.

the parts I see are:

  • given a sequence of number produce the next one
    • partition-all the seq into groups of consecutive numbers
    • for each group of consecutive numbers say how many there are (map say list-o-groups)
    • flatten the result into one list. but really you should use mapcat in the previous step and skip this step.
  • apply the the result to the same function (the iterate function)
  • take as many of these as you need
share|improve this answer
1  
Bluggghhhhh map followed by flatten is vile, don't recommend it. Just use mapcat. – amalloy Sep 2 '11 at 22:57
    
yes, mapcat is better. It makes for a less clear explanation of "separating concerns". I will edit to note this :) – Arthur Ulfeldt Sep 3 '11 at 0:12

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.