Mejorando mi primer programa Clojure

Question

Después de algunos fines de semana explorando Clojure, se me ocurrió este programa. Te permite mover un pequeño rectángulo en una ventana. Aquí está el código:

(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)) 

pesar de ser un programador de C++ con experiencia Me pareció muy difícil de escribir incluso una sencilla aplicación en un idioma que utiliza un estilo radicalmente diferente a lo que estoy acostumbrado.

Además de eso, este código probablemente es una mierda. Sospecho que la globalidad de los diversos valores es algo malo. Tampoco es claro para mí si es apropiado usar referencias aquí para los valores xey.

Cualquier sugerencia para mejorar este código es bienvenido.

Answer 1

Esos if s en keyPressed se pueden reemplazar por un solo case. Además, el dosync se puede mover al exterior para envolver el case. De hecho, alter se puede mover también, por lo que si p. decida cambiarlo a commute, solo hay un lugar para hacer el cambio. El resultado:

(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]))) 

También podría reescribir las importaciones de manera más concisa:

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

- ya sea que querría es una cuestión de estilo.

Cambiaría el nombre a drawRectangle por draw-rectangle (ese es el estilo idiomático para los nombres de funciones en Clojure) y, más concretamente, lo reescribo como una función pura que acepta las coordenadas como argumentos explícitos. Luego, podría escribir un pequeño contenedor para usar sus Refs, si es que su diseño se beneficiaría del uso de Refs. (Es difícil de decir sin saber cómo es posible que desee utilizar & las modifican etc.)

Prefiero while a (loop [] ... (recur)) (ver (doc while) y (clojure.contrib.repl-utils/source while)).

Además, y esto es importante, no coloque nada excepto las definiciones en el nivel superior. Esto se debe a que los formularios de nivel superior se ejecutan cuando se compila el código (intente cargar una biblioteca con un (println :foo) en el nivel superior). Ese bucle infinito debería envolverse dentro de una función; el nombre estándar para una función "principal" en Clojure es -main; lo mismo vale para panel y frame. Esto no se aplica cuando se juega en el REPL, por supuesto, pero es importante saberlo.

Por cierto, (doto foo ...) devuelve foo, por lo que puede escribir (doto (proxy ...) (.setFocusable true) ...).

De lo contrario, diría que el código está bien. Normalmente querrías ponerlo en un espacio de nombres; entonces todas las importaciones irían en el formulario ns.

HTH

Answer 2

Aunque no exactamente consejos Clojure - Considere el uso de KeyAdapter en lugar de KeyListener. De esta forma, no tendrá que proporcionar implementaciones vacías para keyReleased y keyTyped. En general, es una buena regla usar clases de adaptadores cuando no se necesitan todos los métodos de una interfaz de Listener.

Answer 3

Editar, mientras escribo la publicación de abajo con mucha prisa, me olvidé de decir que no debe poner paréntesis alrededor de las constantes, por ejemplo, java.awt.Color/WHITE.

Además de los comentarios de Michal:

• cuando se utiliza un bucle infinito, acondicionarlo con un átomo (por ejemplo @switch) por lo tanto usted será capaz de detenerlo (a menos que los bucles se ejecuta en el hilo repl - así que en un hilo de separación con "futuro")

• utiliza refs, por lo que significa que los valores de xey están destinados a coordinarse (en su código no están: cada actualización es independiente); por lo tanto, en drawRectangle debes ajustar las lecturas en dosync para asegurarte de que obtienes una vista coherente. De nuevo, en tu código actual no importa porque xey siempre se actualizan de forma independiente.

HTH,

(enchufe descarado: http://conj-labs.eu/)