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

Allow null values to be cached and refine TryGet method (see #155) #157

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

EnricoMassone
Copy link

@EnricoMassone EnricoMassone commented Jun 20, 2021

I have introduced the following changes:

  • now the IAppCache.Add method allows to cache null values. No exception is thrown.
  • changed the signature of IAppCache.TryGetValue from bool TryGetValue<T>(string key, out object value); to bool TryGetValue<T>(string key, out T value);
  • modified the implementation of IAppCache.TryGetValue so that values cached by using IAppCache.GetOrAdd and / or IAppCache.GetOrAddAsync are handled correctly (the actual cached value is returned insted of the lazy wrapper).
  • augmented the IAppCache interface with the method bool TryGetValueAsync<T>(string key, out Task<T> value); used to try fetch values cached by using IAppCache.GetOrAddAsync

See issue #155 for the details.

@EnricoMassone
Copy link
Author

Using the IAppCache.TryGetValueAsync method is quite convoluted.

Consider this example:

var cache = new CachingService();

const string key = "my-key";
await cache.GetOrAddAsync(key, async () => 
{
  await Task.Delay(100);
  return "cached value";
}

_ = cache.TryGetValueAsync(key, out var taskForFetchedValue);
var fetchedValue = await taskForFetchedValue; // fetched value is the string "cached value"

The API used must await the Task<T> obtained via the out parameter. Unfortunately we can't await the task for him, by providing an async implementation of IAppCache.TryGetValueAsync, because out parameters are not allowed in async methods.

I'm not a big fan of output parameters, because they are basically parameters used to return values outside of a method. Some people consider them as a bad practice. We can maybe consider to refactor the signature of both IAppCache.TryGetValue and IAppCache.TryGetValueAsync, so that tuples are used instead of out parameters:

 (bool wasKeyFound, T value) TryGetValue<T>(string key);
 Task<(bool wasKeyFound, T value)> TryGetValueAsync<T>(string key);

By doing so, we can write an async implementation of TryGetValueAsync and await the task on behalf of the API user.

@EnricoMassone EnricoMassone changed the title Allow null values to be cached and refine TryGet methods (see #155) Allow null values to be cached and refine TryGet method (see #155) Jun 20, 2021
@EnricoMassone
Copy link
Author

Hi, any news on this ? Did you take a canche to review ?

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.

1 participant