This is related to the thread titled "Proxying in Clojure". I've spent
a lot of time studying the code in this "snake" example and have some
observations I'd like to share.
First off, obviously I care about the future of Clojure or I wouldn't
have spent the last six weeks or so trying to learn it.
I imagine that almost everyone on this mailing list would like to have
the opportunity to spend more time on the job coding in Clojure than
they get to currently. One way to make that more likely is to make it
easier for others to learn Clojure. You may not be allowed to use it
for production code if you're the only one in your company that can
understand it.
Many examples of Clojure code work counter to this goal. This snake
code is just one of them. I don't mean to pick on the authors. This
code is very similar to other Clojure code I encounter daily.
Here are some things I think we could all do in our Clojure code to
make it easier for others to understand.
1) There are no prizes for using one letter variable names. Pick more
meaningful names.
For example, (defn collision? [{[b] :body} a] ...).
What is a? It's a vector containing the x/y coordinates of the apple
the snake is trying to eat. Why not name it "apple" or
"apple-location"?
What is b? It's a vector that represents the x/y coordinates of the
head of the snake.
Why not name it "head" or "snake-head"?
2) There are no prizes for writing code with zero comments. One of the
strengths of Clojure is that you can accomplish a large amount in a
small amount of code. That doesn't mean that readers of your code will
know what is going on though.
For example, what does this do?
(every? #(<= (- (a %) 10) (b %) (+ 10 (a %))) [0 1])
Well, 0 and 1 are indexes of the x and y coordinates represented by
two element vectors that represent the location of the apple and the
head of the snake. This tests whether every coordinate (the x and y)
of the apple are "close" to the corresponding coordinate of the snake
head. This certainly needs to be explained in a comment.
3) There are no prizes for cramming loads of functionality into a
single line. Spread it out to make it easier to read.
For example, which of these is easier to understand?
; original
(assoc snake :body (cons (vec (map #(+ (dir %) ((first body) %)) [0 1]))
(if grow body (butlast body)))))
What is being cons'ed to what here?
; modified
(assoc snake :body
(cons
(vec (map #(+ (dir %) ((first body) %)) [0 1]))
(if grow body (butlast body))
)
)
I know my placement of the closing parens on separate lines is
non-standard in the Lisp world, but I find it helps me see better
where constructs end. Without doing that it feels like Python where
indentation is significant. I could concede the paren placement
though.
Below is the original code. Compare it to my version at
http://pastie.org/348031 where variables are renamed, comments are
added, and indentation is changed in a way that I feel makes it more
readable.
(import '(java.awt Color) '(javax.swing JPanel JFrame Timer)
'(java.awt.event KeyEvent ActionListener KeyListener))
(defn gen-apple [_] [(rand-int 750) (rand-int 550)])
(defn move [{:keys [body dir] :as snake} & grow]
(assoc snake :body (cons (vec (map #(+ (dir %) ((first body) %)) [0 1]))
(if grow body (butlast body)))))
(defn turn [snake newdir] (if newdir (assoc snake :dir newdir) snake))
(defn collision? [{[b] :body} a]
(every? #(<= (- (a %) 10) (b %) (+ 10 (a %))) [0 1]))
(defn paint [g p c] (.setColor g c) (.fillRect g (p 0) (p 1) 10 10))
(def dirs {KeyEvent/VK_LEFT [-10 0] KeyEvent/VK_RIGHT [10 0]
KeyEvent/VK_UP [0 -10] KeyEvent/VK_DOWN [0 10]})
(def apple (atom (gen-apple nil)))
(def snake (atom {:body (list [10 10]) :dir [10 0]}))
(def colors {:apple (Color. 210 50 90) :snake (Color. 15 160 70)})
(def panel (proxy [JPanel ActionListener KeyListener] []
(paintComponent [g] (proxy-super paintComponent g)
(paint g @apple (colors :apple))
(doseq [p (:body @snake)]
(paint g p (colors :snake))))
(actionPerformed [e] (if (collision? @snake @apple)
(do (swap! apple gen-apple)
(swap! snake move :grow))
(swap! snake move))
(.repaint this))
(keyPressed [e] (swap! snake turn (dirs (.getKeyCode e))))
(keyReleased [e])
(keyTyped [e])))
(doto panel (.setFocusable true)(.addKeyListener panel))
(doto (JFrame. "Snake")(.add panel)(.setSize 800 600)(.setVisible true))
(.start (Timer. 75 panel))
Another example of a fairly short piece of code that is begging for
comments is the example at http://clojure.org/concurrent_programming.
Imagine someone who has only been learning Clojure for a couple of
days and is curious about how Clojure handles concurrency trying to
follow what is happening in that code.
So my main point is that if we all make an effort to make our code
easier to read, it will be easier to convince other developers to
consider learning Clojure which will be good for all of us.
--
R. Mark Volkmann
Object Computing, Inc.
--~--~---------~--~----~------------~-------~--~----~
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
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
-~----------~----~----~----~------~----~------~--~---
