-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: implement SetIfPresent #12
Conversation
Could you please provide some example, why would you want to do that? And as far as you extend public iface, it probably should be represented in readme |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks.
Updated the readme!
without |
.gitignore
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor: I believe, that developer related stuff (ide settings, personal tooling, etc.) should be ignored in git config --global
, not in the project .gitignore
. It helps maintain simple, clean, and easy to read and understand ignore file. Project .gitignore
could contain some build artifacts, or dot-secrets, or smth like so.
doc.go
Outdated
@@ -9,6 +9,8 @@ var ErrNotFound = errors.New("not found") | |||
// Geche interface is a common interface for all cache implementations. | |||
type Geche[K comparable, V any] interface { | |||
Set(K, V) | |||
// SetIfPresent sets the kv only if the key was already present, and returns whether the insertion was performed | |||
SetIfPresent(K, V) bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I just realised that it would be a shame to implement atomic Get/Set without returning the previous value. This will increase the number of possible use cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, I'll do this later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refactored
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
@C-Pro when you have time, could you please merge this and create a new package version? I will likely need this next week |
Added to all implementations and most tests
SetIfPresent
sets the provided key to the provided value if it already exists in the cache implementation. ForMapTTLCache
, it also reinitializes the TTL