Hi, >From a cursory glance, I didn't really understand the domain, so the function names I used in my rewrite might seem silly. But I wanted to illustrate that there is a lot of repetition in your code. Also discrete functions (with proper names) can make the code easier to grok.
https://gist.github.com/amithgeorge/15b1cb607c32d39b70c7 ;; instead of relying on the caller to pass the edges in the right order, ;; we accept a map and process the keys in the correct order (defn- squared-edge-values [edges] (->> [:oc :oa :ob :ca :ab :bc] (map #(edges %1)) (map #(* %1 %1)))) ;; all operations on the edges are performed on the squared values. ;; the squared values are guaranteed to be in the correct order. ;; this separates out the logic specific to opposite edges and can be tested separately (defn- opposite-edge-values [edges] (let [[a2 b2 c2 d2 e2 f2] (squared-edge-values edges)] [[a2 e2 (+ a2 e2)] [b2 f2 (+ b2 f2)] [c2 d2 (+ c2 d2)]])) (defn- closed-edge-values [edges] (let [[a2 b2 c2 d2 e2 f2] (squared-edge-values edges)] [[a2 b2 d2] [d2 e2 f2] [b2 c2 e2] [a2 c2 f2]])) (defn- open-edge-values [edges] ;; implement later [[0 0 0]]) (defn- compute-edge-values [edge-fn edges] (->> edges (edge-fn) (map (fn [[x y z]] (* x y z))) (reduce +))) (def ^:private add-opposite (partial compute-edge-values opposite-edge-values)) (def ^:private add-closed (partial compute-edge-values closed-edge-values)) (def ^:private add-open (partial compute-edge-values open-edge-values)) ;; this is the public entry point. ;; the precondition ensures all the edges are present in the input (defn volume [{:keys [oa ob oc ab bc ca] :as edges}] {:pre [(every? #(number? %1) [oa ob oc ab bc ca])]} (let [opposite (add-opposite edges) closed (add-closed edges) open (add-open edges)] (Math/sqrt (* (- (- open closed) opposite) 0.5)))) ;; sample input (defn a-mod-input [] (let [a 1.0] {:oc (* a (/ (Math/sqrt 6.0) 12.0)) :oa (* a (/ (Math/sqrt 6.0) 4.0)) :ob (* a (/ (Math/sqrt 2.0) 4.0)) :ca (* a (/ (Math/sqrt 3.0) 3.0)) :ab (/ a 2.0) :bc (* a (/ (Math/sqrt 3.0) 6.0))})) (defn e-mod-input [] (let [d 1.0 r (/ d 2.0) h r] {:oc h :oa 1 :ob 1 :ca 1 :ab 1 :bc 1})) (def a-mod-volume (volume (a-mod-input))) (def e-mod-volume (volume (e-mod-input))) Again, the idea is to have small functions that reflect the steps you would normally do, as required by the domain or algo you are implementing. -- 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 Note that posts from new members are moderated - please be patient with your first post. 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 --- You received this message because you are subscribed to the Google Groups "Clojure" group. To unsubscribe from this group and stop receiving emails from it, send an email to clojure+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
