RokLenarcic / memento

Clojure Memoization project
MIT License
33 stars 2 forks source link

memo/memo doesn't work as expected? #3

Open kanwei opened 2 months ago

kanwei commented 2 months ago

Thanks for the library!

We're trying to replace core.memoize with this library, but some of the behavior is unexpected:

(def memo-rand-int (memo/memo (fn [] (rand-int 1000)) {mc/cache mc/caffeine}) )

(memo-rand-int)
=> 359
(memo-rand-int)
=> 999
(memo-rand-int)
=> 778

Doesn't seem to actually return a memoized function like core.memoize would.

Have to explicitly call defmemo:

(memo/defmemo memo-rand-int-defmemo
              {mc/cache {mc/type mc/caffeine}}
              []
              (rand-int 1000))

(memo-rand-int-defmemo)
=> 69
(memo-rand-int-defmemo)
=> 69
(memo-rand-int-defmemo)
=> 69

Was wondering what the use case for memo/memo is. Thanks!

RokLenarcic commented 2 months ago

You are using the wrong property. The config map should be {mc/type mc/caffeine} not {mc/cache mc/caffeine}. mc/cache is the keyword for function meta if you're placing the configuration into the function meta. So:

(def memo-rand-int (memo/memo (fn [] (rand-int 1000)) {mc/type mc/caffeine}) )

or

(defn memo-rand-int {mc/cache {mc/type mc/caffeine}} [] (rand-int 1000)))

(memo/memo #'memo-rand-int)
RokLenarcic commented 2 months ago

I'll add a check to throw an error if mc/cache is passed in cache configuration as that's clearly a misuse.

kanwei commented 2 months ago

You'll have to fix the docs too since the README docs are literally wrong too:

Caching an anonymous function
You can add cache to a function object (in clojure.core/memoize fashion):

(m/memo (fn [] ...) {mc/cache mc/caffeine})
RokLenarcic commented 2 months ago

My bad

RokLenarcic commented 2 months ago

Fixed Docs