Skip to content
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

Add match option to Nebulex.Caching #56

Merged
merged 2 commits into from
Oct 21, 2019
Merged

Conversation

polmiro
Copy link
Contributor

@polmiro polmiro commented Oct 21, 2019

Sometimes one wants to conditionally cache a value based on the result of the underlying function logic. This is easy to achieve by using the lower-level APIs Cache.get and Cache.set but it is fairly verbose and repeatable. This PR adds a new option to the Nebulex.Caching DSL to support it. For example, a classic use case would be avoiding caching the result of a REST API when a transient error is returned.

This PR adds a new option :match that takes a function with one argument. The function will be called with the value in the cache and the cache will be updated based on the boolean result returned.

@coveralls
Copy link

coveralls commented Oct 21, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 6ac0560 on polmiro:cache-with-match into 48215f3 on cabol:master.

lib/nebulex/caching.ex Outdated Show resolved Hide resolved
@cabol
Copy link
Owner

cabol commented Oct 21, 2019

@polmiro only one silly comment, other thank that, very very nice job man, thank you very much for your contribution, this is a very cool feature 😃 👍

@polmiro
Copy link
Contributor Author

polmiro commented Oct 21, 2019

Thinking now about naming, I am wondering if the option should be named if instead of match? What do you think?

@cabol
Copy link
Owner

cabol commented Oct 21, 2019

Thinking now about naming, I am wondering if the option should be named if instead of match? What do you think?

I like match, I think it is clear enough and self-descriptive, we can keep it like that for now, until we come up with a better idea I guess 😄

@polmiro
Copy link
Contributor Author

polmiro commented Oct 21, 2019

Sounds good! It's ready then :)

@cabol cabol merged commit 21f21e3 into cabol:master Oct 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants