Clojure GUI calculator problem. "-" always returns 0 and "/" always returns 1.

Question

I'm well aware this code is horrible. I just made it so I could try out different Swing stuff. This is an interesting problem. This creates 4 buttons one for adding, subtracting, dividing and multiplying. Multiplying and adding works perfectly, no problems at all, but when the user tries to subtract it always returns 0. When the user tries to divide it always returns 1 or 1.0 depending on input. I can't figure it out. Here is the code.

(ns rayne.main
  (:gen-class)
  (:import (javax.swing JFrame JTextField JButton JOptionPane)
           (java.awt.event ActionListener)
           (java.awt GridLayout)))

(def numbers (ref []))
(def times-clicked (ref 0))

(defn calc [nseq op]
  (let [n1 (first nseq)
        n2 (last nseq)]
  (cond
    (= op "+") (+ n1 n2)
    (= op "*") (* n1 n2)
    (= op "-") (- n2 n1)
    (= op "/") (/ n1 n2))))

(defn add-op-button [op text button]
  (.addActionListener button
    (proxy [ActionListener] []
      (actionPerformed [e]
      (dosync
       (ref-set times-clicked (inc @times-clicked))
       (if (= @times-clicked 2)
         (do
         (let [result (.toString (calc @numbers op))
               result2 (read-string result)]
           (.setText text result)
           (ref-set numbers [])
           (ref-set times-clicked 0)))
       (do
         (ref-set numbers (conj @numbers (read-string (.getText text))))
         (.setText text ""))))))))

(defn -main []
  (let [frame (JFrame. "Calculator")
        add-button (JButton. "+")
        sub-button (JButton. "-")
        mul-button (JButton. "*")
        div-button (JButton. "/")
        clr-button (JButton. "Clear")
        text-field (JTextField.)]
    (add-op-button "+" text-field add-button)
    (add-op-button "-" text-field sub-button)
    (add-op-button "*" text-field mul-button)
    (add-op-button "/" text-field div-button)
(doto frame
  (.setLayout (GridLayout. 1 5))
  (.add text-field)
  (.add add-button)
  (.add sub-button)
  (.add mul-button)
  (.add div-button)
  (.setSize 500 100)
  (.setVisible true))))

The indentation is probably screwed up, because of copy-pasting and on the fly formatting, but there it is. Once again I know the code is horrible.

Answer 1

The second time the user clicks a button, the number isn't added to the list numbers, so you're doing calc on a list of one element.

Since the list has only one element, the first element (n1) and the last element (n2) are the same thing, and

x / x => 1
x - x => 0

I'm surprised your addition and multiplication are working... :-/

I think you can fix this by moving the update of @numbers to before the if:

(ref-set numbers (conj @numbers (read-string (.getText text))))
(if (= @times-clicked 2)

or else move the increment of @times-clicked to be after the if.

Answer 2

It looks like add is only ever adding the number to itself, and the multiplication function is actually just squaring the first number.