Encapsulated state in clojure

Question

While going through SICP and trying to implement the code in clojure, I've found that while I can get the code in chapter 3 to work, it seems to go against Clojure idioms, but I can't quite imagine the "Clojure way" to do it either.

Take for example, the digital circuit simulator. This is my implementation:

(declare call-each)
(defn make-wire []
 (let [signal-value (atom 0)
 action-procedures (atom '())
 set-my-signal (fn [new-value]
 (if-not (= @signal-value new-value)
 (do (reset! signal-value new-value)
 (call-each @action-procedures))
 'done))
 accept-action-procedure! (fn [proc]
 (swap! action-procedures
 conj proc)
 (proc))
 dispatch (fn [m]
 (cond (= m 'get-signal) @signal-value
 (= m 'set-signal) set-my-signal
 (= m 'add-action) accept-action-procedure!
 :else (error "Unknown operation -- WIRE" m)))]
 dispatch))
(defn call-each [[proc & rest :as procedures]]
 (if (empty? procedures)
 'done
 (do (proc)
 (call-each rest))))
(defn get-signal [wire]
 (wire 'get-signal))
(defn set-signal! [wire new-value]
 ((wire 'set-signal) new-value))
(defn add-action! [wire action-procedure]
 ((wire 'add-action) action-procedure))

As you can see, it has an internal signal-value and action-procedures as state. Is this the natural way to do this in Clojure, and if not, how would you do it?

• toto2: It would be possible to make the wire truly immutable by returning a new wire when changing the state. However, it's not clear how the wire relates to the rest of the system. Probably the whole system would to be recreated when changing the state of a wire, which might be bad for performance.

Answer 1

There are a few problems.

• You should not read the value from an atom and update it conditional on its value, as in set-my-signal. It's value may have changed in between those two calls. In this case refs instead of atoms may need to be used.

• Do not use unbounded recursion as in call-each.

Instead of:

(defn call-each [[proc & rest :as procedures]]
 (if (empty? procedures)
 'done
 (do (proc)
 (call-each rest))))

You can do:

(defn- call-each [procedures]
 (doseq [proc procedures] (proc)))

• error is not a clojure function. So you should have implemented in clojure with throw... Just use throw instead. It would read something like: (throw (UnsupportedOperationException. (str "Unknown operation -- WIRE " m) nil)). Or instead of throwing an exception you can return nil. So it would be easier to extend (add new messages by decorating) the make-wire constructor.

• Case analysis (cond) on a constant symbols is someone does a lot in Scheme. Clojure maps are more readable though. Also using keywords :get-signal instead of symbols 'get-signal is more conventional in accessing the fields of a map, record, so on..

make-wire would be read something like this, if I wrote it in Clojure:

(defn make-wire []
 (let [signal-value (ref 0)
 action-procedures (ref '())]
 {
 :get-signal (fn [] @signal-value)
 :set-signal (fn [new-value]
 (dosync
 (when-not (= @signal-value new-value) 
 (ref-set signal-value new-value)
 (call-each @action-procedures))))
 :add-action (fn [proc]
 (dosync
 (alter action-procedures conj proc)
 (proc)))
 }))

The point of encapsulation as taught in SICP is that you can combine components independent of their implementation details.

So you might want to define a protocol explicitly for a wire:

(defprotocol Wire
 (get-signal [w])
 (set-signal! [w new-value])
 (add-action! [w action-procedure]))

Then whether you implement it using a record:

(defrecord WireRec [signal-value action-procedures]
 Wire
 (get-signal [_] @signal-value)
 (set-signal! [_ new-value]
 (dosync
 (when-not (= @signal-value new-value)
 (ref-set signal-value new-value)
 (call-each @action-procedures))))
 (add-action! [_ proc]
 (dosync
 (alter action-procedures conj proc)
 (proc))))
(defn make-wire2 []
 (WireRec. (ref 0) (ref '())))

Or a map:

(defn make-wire3 []
 (let [m (ref {
 :signal-value 0
 :action-procedures '()
 })]
 (reify Wire
 (get-signal [_] (:signal-value @m))
 (set-signal! [_ new-value]
 (dosync
 (when-not (= (:signal-value @m) new-value)
 (alter m assoc :signal-value new-value)
 (call-each (:action-procedures @m)))))
 (add-action! [_ proc]
 (dosync
 (alter m update-in [:action-procedures] conj proc)
 (proc))))))

should not matter.