diff --git a/src/robertluo/fun_map/core.cljc b/src/robertluo/fun_map/core.cljc index 3df1cd1..dbf3757 100644 --- a/src/robertluo/fun_map/core.cljc +++ b/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 [_] diff --git a/src/robertluo/fun_map/wrapper.cljc b/src/robertluo/fun_map/wrapper.cljc index 93f59ef..ce20cd3 100644 --- a/src/robertluo/fun_map/wrapper.cljc +++ b/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 "<>"))))) diff --git a/test/robertluo/fun_map/core_test.cljc b/test/robertluo/fun_map/core_test.cljc index 8bb42ee..4459b88 100644 --- a/test/robertluo/fun_map/core_test.cljc +++ b/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)))