Улучшение моей первой программы Clojure

После нескольких выходных, изучающих Clojure, я придумал эту программу. Это позволяет вам перемещать маленький прямоугольник в окне. Здесь код:

(import java.awt.Color)
(import java.awt.Dimension)
(import java.awt.event.KeyListener)
(import javax.swing.JFrame)
(import javax.swing.JPanel)

(def x (ref 0))
(def y (ref 0))

(def panel
  (proxy [JPanel KeyListener] []
    (getPreferredSize [] (Dimension. 100 100))
    (keyPressed [e]
      (let [keyCode (.getKeyCode e)]
        (if (== 37 keyCode) (dosync (alter x dec))
        (if (== 38 keyCode) (dosync (alter y dec))
        (if (== 39 keyCode) (dosync (alter x inc))
        (if (== 40 keyCode) (dosync (alter y inc))
                            (println keyCode)))))))
    (keyReleased [e])
    (keyTyped [e])))

(doto panel
  (.setFocusable true)
  (.addKeyListener panel))

(def frame (JFrame. "Test"))
(doto frame
    (.add panel)
    (.pack)
    (.setDefaultCloseOperation JFrame/EXIT_ON_CLOSE)
    (.setVisible true))

(defn drawRectangle [p]
  (doto (.getGraphics p)  
    (.setColor (java.awt.Color/WHITE))
    (.fillRect 0 0 100 100)
    (.setColor (java.awt.Color/BLUE))
    (.fillRect (* 10 (deref x)) (* 10 (deref y)) 10 10)))

(loop []
  (drawRectangle panel)
  (Thread/sleep 10)
  (recur))

Несмотря на то, что я был опытным программистом на С++, мне было очень сложно писать даже простое приложение на языке, который использует совершенно иной стиль, чем то, к чему я привык.

Кроме того, этот код, вероятно, сосет. Я подозреваю, что глобальность различных ценностей - это плохо. Мне также непонятно, следует ли использовать ссылки здесь для значений x и y.

Любые советы по улучшению этого кода приветствуются.

Ответ 1

Те if в keyPressed могут быть заменены одним case. Кроме того, dosync можно перемещать за пределы, чтобы обернуть case. Фактически, alter также может быть перемещен, так что, если вы, например, решите изменить его на commute, там только одно место, чтобы внести изменения. Результат:

(def panel
  (proxy [JPanel KeyListener] []
    (getPreferredSize [] (Dimension. 100 100))
    (keyPressed [e]
      (let [keyCode (.getKeyCode e)]
        (dosync
         (apply alter
           (case keyCode
             37 [x dec]
             38 [y dec]
             39 [x inc]
             40 [y inc])))
        (println keyCode)))
    (keyReleased [e])
    (keyTyped [e])))

Вы также можете переписать импорт более кратко:

(import [java.awt Color Dimension event.ActionListener])
(import [javax.swing JFrame JPanel])

- хотите ли вы, это вопрос стиля.

Я бы переименовал drawRectangle в draw-rectangle (это идиоматический стиль для имен функций в Clojure) и, более того, переписал его как чистую функцию, принимающую координаты как явные аргументы. Тогда вы можете написать небольшую обертку вокруг этого, чтобы использовать ваши Refs, если бы ваш дизайн выиграл от использования Ref. (Трудно сказать, не зная, как вы можете использовать и изменять их и т.д.)

Предпочитают while - (loop [] ... (recur)) (см. (doc while) и (clojure.contrib.repl-utils/source while)).

Также - и это важно - не помещать ничего, кроме определений на верхнем уровне. Это потому, что формы верхнего уровня фактически выполняются при компиляции кода (попробуйте загрузить библиотеку с (println :foo) на верхнем уровне). Этот бесконечный цикл должен быть обернут внутри функции; стандартное имя для "основной" функции в Clojure - -main; то же относится к panel и frame. Это не относится, если вы играете в REPL, конечно, но это важный вопрос, о котором нужно знать.

Кстати, (doto foo ...) возвращает foo, поэтому вы можете просто написать (doto (proxy ...) (.setFocusable true) ...).

В противном случае, я бы сказал, что код в порядке. Обычно вы хотите поместить его в пространство имен; то весь импорт будет в форме ns.

НТН

Ответ 2

Редактирование, при написании нижеприведенного сообщения в спешке я забыл сказать, что вы не должны помещать парс вокруг констант, например java.awt.Color/WHITE.

В дополнение к комментариям Michał:

когда вы используете бесконечный цикл, задайте его с помощью атома (например, @switch), таким образом, вы сможете остановить его (если петли не выполняются на реплике), так что это будет в разделительной нити с "будущим" )
вы используете refs, поэтому это означает, что значения x и y должны быть скоординированы (в вашем коде они отсутствуют: каждое обновление является независимым); поэтому в drawRectangle вы должны обернуть чтение в dosync, чтобы убедиться, что вы получаете согласованное представление - снова в вашем фактическом коде это не имеет значения, потому что x и y всегда обновляются независимо.

HTH,

(бесстыдный плагин: http://conj-labs.eu/)

Ответ 3

Хотя не точно Clojure advice - рассмотрите возможность использования KeyAdapter вместо KeyListener. Таким образом вам не придется предоставлять пустые реализации для keyReleased и keyTyped. Как правило, это хорошее правило для использования классов адаптера, когда вам не нужны все методы интерфейса Listener.