-
Notifications
You must be signed in to change notification settings - Fork 120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support polymorphic keys #355
Comments
Love the idea of getting rid of Are you thinking that we will also provide auto/semi-auto derivation for instances of that type class? I’m guessing that it’s something we’d want to do, but also may be out of the scope of this issue and we could revisit after we finish this one. Sent with GitHawk |
Hi,
I'm thinking the cleanest way to do that might be to take the memoization methods out of the main trait hierarchy and separate them to some side trait that could be used as some sort of decorator when memoization is wanted and the key type is string, maybe as a self type. What do you think? |
Made a pull request with my changes, I hope it's alright. Let me know what you think. |
Currently we only support
String
as a cache key. This makes sense for remote caches such as Redis and Memcached, where the key needs to be a bytestring, but is unnecessarily limiting (and slow) for in-memory caches such as Caffeine.I'd like to update the API to support any type
K
as a cache key.Things to consider:
We need a way to turn a value of type
K
into a representation type that the cache implementation can handle:String
Array[Byte]
K
, assumingK
has a sensibleequals
andhashCode
Probably a
KeyEncoder[K, Repr]
type class would make sense here. e.g.RedisCache[K]
would require aKeyEncoder[K, Array[Byte]]
.The
memoize
macro generates a String key, e.g.myFunction(123, "abc")
. So it will only work with caches that accept String keys.The API currently accepts
keyParts: Any*
, callstoString
on each part and concatenates them together using a configurable separator. I'm not sure how useful this feature really is. Probably better to just takekey: K
and stop messing around with varargs andtoString
.The text was updated successfully, but these errors were encountered: