Improve error handling

src/robertluo/fun_map/core.cljc

@@ -7,7 +7,7 @@
                     ATransientMap])))
 
 #?(:clj
-;;Marker iterface for a funmap
+;; Marker interface for a funmap
    (definterface IFunMap
      (rawSeq []))
    :cljs
@@ -59,7 +59,18 @@
       (->DelegatedMap (-persistent! tm) fn-entry))
      (-conj!
       [_ pair]
-      (TransientDelegatedMap. (-conj! tm pair) fn-entry))))
+      (TransientDelegatedMap. (-conj! tm pair) fn-entry))
+
+     ILookup
+     (-lookup
+      [this k]
+      (-lookup this k nil))
+     (-lookup
+      [this k not-found]
+      (if-let [entry (when (-contains-key? tm k)
+                       (fn-entry this (-find tm k)))]
+        (val entry)
+        not-found))))
 
 #?(:clj
 ;; DelegatedMap takes a map `m` and delegates most feature to it.
@@ -90,13 +101,13 @@
      clojure.lang.IFn
      (invoke [this k] (.valAt this k))
      (invoke [this k not-found] (.valAt this k not-found))
-     clojure.lang.ILookup
-     (valAt [this k]
-       (some-> ^IMapEntry (.entryAt this k) (.val)))
-     (valAt [this k not-found]
-       (if (.containsKey this k)
-         (.valAt this k)
-         not-found))
+    clojure.lang.ILookup
+    (valAt [this k]
+      (some-> ^IMapEntry (.entryAt this k) (.val)))
+    (valAt [this k not-found]
+      (if-let [entry (.entryAt this k)]
+        (.val ^IMapEntry entry)
+        not-found))
      clojure.lang.IPersistentMap
      (count [_]
        (.count m))
@@ -148,6 +159,10 @@
      (putAll [_ _] (throw (UnsupportedOperationException.)))
      (clear [_] (throw (UnsupportedOperationException.)))
 
+     clojure.lang.IKVReduce
+     (kvreduce [this f init]
+       (reduce-kv (fn [acc k _] (f acc k (.valAt this k))) init m))
+
      clojure.lang.IEditableCollection
      (asTransient [_]
        (TransientDelegatedMap. (transient m) fn-entry)))
@@ -182,13 +197,15 @@
      (-invoke [this k] (-lookup this k))
      (-invoke [this k not-found] (-lookup this k not-found))
 
-     ILookup
-     (-lookup
-       [this k]
-       (-lookup this k nil))
-     (-lookup
-       [this k not-found]
-       (or (some-> ^IMapEntry (-find this k) (val)) not-found))
+    ILookup
+    (-lookup
+      [this k]
+      (-lookup this k nil))
+    (-lookup
+      [this k not-found]
+      (if-let [entry (-find this k)]
+        (val entry)
+        not-found))
 
      IMap
      (-dissoc
@@ -251,6 +268,11 @@
      IMeta
      (-meta [_] (-meta m))
 
+     IKVReduce
+     (-kv-reduce
+      [this f init]
+      (reduce-kv (fn [acc k _] (f acc k (-lookup this k))) init m))
+
      IEditableCollection
      (-as-transient
        [_]

src/robertluo/fun_map/wrapper.cljc

@@ -20,11 +20,14 @@
      Object
      (-wrapped? [_ _] false)
      (-unwrap [this _ k]
-       (ex-info "Unwrap a common value" {:key k :value this}))
+       ;; This should never be called since -wrapped? returns false.
+       ;; If it is called, it indicates a bug in the unwrapping logic.
+       (throw (ex-info "Bug: attempted to unwrap a non-wrapper value"
+                       {:key k :value this :type (type this)})))
      nil
      (-wrapped? [_ _] false)
      (-unwrap [_ _ k]
-       (ex-info "Unwrap a nil" {:key k}))
+       (throw (ex-info "Bug: attempted to unwrap nil" {:key k})))
      clojure.lang.IDeref
      (-wrapped? [_ m]
        (not (some-> m meta ::keep-ref)))
@@ -90,7 +93,7 @@
   )
 
 (def fun-wrapper
-  "construct a new FunctionWrapper"
+  "Constructs a new FunctionWrapper."
   ->FunctionWrapper)
 
 (defn- check-cycle!
@@ -134,11 +137,17 @@
   ValueWrapper
   (-wrapped? [_ _] true)
   (-unwrap [_ m k]
-    (let [[val focus-val] @a-val-pair
-          new-focus-val (if focus-fn (focus-fn m) ::unrealized)]
-      (if (or (= ::unrealized val) (not= new-focus-val focus-val))
-        (first (swap! a-val-pair (fn [_] [(-unwrap wrapped m k) new-focus-val])))
-        val)))
+    ;; Use swap! properly to avoid race condition where multiple threads
+    ;; could redundantly recompute the value.
+    ;; Note: focus-fn must be pure since it may be called multiple times
+    ;; during contention.
+    (first
+      (swap! a-val-pair
+        (fn [[val focus-val]]
+          (let [new-focus-val (if focus-fn (focus-fn m) ::unrealized)]
+            (if (or (= ::unrealized val) (not= new-focus-val focus-val))
+              [(-unwrap wrapped m k) new-focus-val]
+              [val focus-val]))))))
   #?@(:cljs
       [IPrintWithWriter
        (-pr-writer
@@ -150,7 +159,7 @@
                      ">>")))]))
 
 (defn cache-wrapper
-  "construct a CachedWrapper"
+  "Constructs a CachedWrapper."
   [wrapped focus]
   (CachedWrapper. wrapped (atom [::unrealized ::unrealized]) focus))
 
@@ -164,7 +173,7 @@
       v)))
 
 (def trace-wrapper
-  "constructs a TraceWrapper"
+  "Constructs a TracedWrapper."
   ->TracedWrapper)
 
 ;; Fine print the wrappers
@@ -178,4 +187,7 @@
                (str "<<"
                     (let [v (-> (.a_val_pair o) deref first)]
                       (if (= ::unrealized v) "unrealized" v))
-                    ">>")))))
+                    ">>")))
+
+     (defmethod print-method TracedWrapper [^TracedWrapper o ^java.io.Writer wtr]
+       (.write wtr (str "<<traced:" (.wrapped o) ">>")))))

test/robertluo/fun_map/core_test.cljc

@@ -15,6 +15,18 @@
       (is (= 2 (m :a)))
       (is (= ::not-found (m :c ::not-found))))))
 
+(deftest nil-and-false-value-test
+  (testing "nil values are returned correctly with not-found"
+    (let [m (sut/delegate-map {:a nil :b 1} (fn [_ [k v]] [k v]))]
+      (is (nil? (get m :a)))
+      (is (nil? (get m :a :not-found))) ; should return nil, not :not-found
+      (is (= :not-found (get m :missing :not-found)))))
+  (testing "false values are returned correctly with not-found"
+    (let [m (sut/delegate-map {:a false :b true} (fn [_ [k v]] [k v]))]
+      (is (false? (get m :a)))
+      (is (false? (get m :a :not-found))) ; should return false, not :not-found
+      (is (= :not-found (get m :missing :not-found))))))
+
 (deftest transient-test
   (letfn [(wrap-m [m f] (-> m (sut/delegate-map (fn [_ [k v]] [k (* 2 v)])) transient f persistent!))]
     (is (= {} (wrap-m {} identity)))