views:

159

answers:

3

Here is a bit of example code:

(deftype Deck52 [suits] :as this
  DeckOfCards
  (check-empty []
               (Deck52. (apply hash-map 
                              (apply concat (remove (-> nil?) 
                                                    (for [[key val] suits] 
                                                      (if (empty? val) nil [key val])))))))
  (remove-card [suit card]
               (assoc suits suit (remove #(= card %) (suit suits))))

  (get-card [suit]
            (let [suitd (suit suits)]
              [(first suitd) (check-empty (Deck52. (assoc suits suit (rest suitd))))]))

  (random-card []
               (let [suitn (+ 1 (rand-int 4))]
                 (cond (= suitn 1) (get-card this :hearts)
                       (= suitn 2) (get-card this :diamonds)
                       (= suitn 3) (get-card this :clubs)
                       (= suitn 4) (get-card this :spades)))))

I also posted this code in a Gist here if it's easier to read: http://gist.github.com/307425 (not going anywhere).

The primary example here is check-empty. I had great difficulty knowing where I should and shouldn't hit return, and I still don't know if I did it right. It threatens to walk right off the right side of the screen, but that's the way clojure-mode indents, and I assume that it's supposed to be like that.

So, the question is, when is it time to place a newline in Clojure/Lisp code? Am I 'doin' it rite'?

NOTE: I can't promise that the code I posted works. I've been doing some experimentation, and some things might be plain ol' sucky, if not broken.

+1  A: 

when things are trying to escape off the right hand side I tend to his enter after the first argument to each call. this only indents by a fixed amount per line.

(check-empty []
             (Deck52. 
              (apply 
               hash-map 
               (apply 
                concat 
                (remove 
                 (-> 
                  nil?) 
                  (for [[key val] suits] 
                   (if (empty? val) nil [key val])))))))
Arthur Ulfeldt
+6  A: 

The way you broke the lines is quite normal. I would do some slight changes though.

  • Put the argument vectors on the next line.
  • Use different forms: the little -> and ->> helpers, condp, when, ...
  • If necessary, break the line immediately after the function name.

Here how I would go about this. (Disclaimer: My style. Yours might be different. YMMV!)

(deftype Deck52 [suits] :as this
  DeckOfCards
  (check-empty
    []
    (->> (for [[key val] suits]
           (when-not (empty? val)
             [key val]))
      (remove nil?)
      (apply concat)
      (apply hash-map)
      Deck52.))
  (remove-card
    [suit card]
    (assoc suits suit (remove #(= card %) (suit suits))))
  (get-card
    [suit]
    (let [suitd (suit suits)]
      [(first suitd)
       (->> (rest suitd)
         (assoc suits suit)
         Deck52.
         check-empty)]))
  (random-card
    []
    (let [suitn (+ 1 (rand-int 4))]
      (condp = suitn
        1 (get-card this :hearts)
        2 (get-card this :diamonds)
        3 (get-card this :clubs)
        4 (get-card this :spades)))))

Although the following is not part of your question, I cannot resist:

(deftype Deck52 [suits] :as this
  DeckOfCards
  (check-empty
    []
    (->> suits (remove (comp nil? seq val)) (into {}) Deck52.))
  (remove-card
    [suit card]
    (update-in suits [suit] #(remove %2 %1) #(= card %)))
  (get-card
    [suit]
    (let [suitd (get suits suit)]
      [(first suitd)
       (->> (rest suitd) (assoc suits suit) Deck52. check-empty)]))
  (random-card
    []
    (case (rand-int 4)
      0 (get-card this :hearts)
      1 (get-card this :diamonds)
      2 (get-card this :clubs)
      3 (get-card this :spades))))
kotarak
I just love finding out my code sucks. Thanks for the answer. You've shown me a few things I didn't know about, and inadvertently taught me how to use -> and ->>. :(
Rayne
Are you on the Clojure IRC by any chance? If so, what name do you go under?
Rayne
Well. Your code doesn't suck. It's just verbose. That's normal, I guess. I - for my part - always forget about `zipmap`, for example. I'm on #clojure as `kotarak`. Normally around 9pm to 10pm CET (GMT+1h, I think).
kotarak
No, it sucked pretty bad, but thanks for trying to make me feel better. I think that, armed with -> and ->>, I'll be able to avoid such obvious mistakes in the past.
Rayne
This question and the answer show a very interesting progression. Without even reading the code in detail you can see the stepwise increase in familiarity with the language facilities and idioms. Thank you both for the question and answer.
clartaq
+2  A: 

clojure-mode for Emacs doesn't necessarily get all the indentations right. I say try to keep your lines under 80 characters and be consistent.

Stuart Sierra