Open MNF opened 5 years ago
Thanks for the suggestion, here are some thoughts:
IObjectCache - the interface from caching abstractions we depend on in v2 - offers 2 methods
Historically LazyCache has always just offered the "Set" behaviour (there are not many reasons why you would want to not cache something if it was already cached), but it is implemented in a method called Add() because the old version of the API was named that way, in the old packages we used to depend on.
In general, I am not a fan of having two API methods that do the same thing with different name as it adds confusion to users. But maybe there is a case to suggest we should deprecate the Add()
method and direct users to a new Set()
method, and give users a time to be able to switch over?
However, this is almost a breaking change (some users have warnings as build stoppers) and so I am inclined to leave this issue here for a while to see if others think this is worthwhile or not.
I think at the very least, it should be documented in the method (XML comments). I had to find this ticket to figure out if it was going to throw an exception or if it might silently not update the cached object.
I think at the very least, it should be documented in the method (XML comments). I had to find this ticket to figure out if it was going to throw an exception or if it might silently not update the cached object.
@junweilee fancy adding some nice XML comments to CachingService.IAppCache?
I propose we rename the method to Set<>(...)
and but leave Add<>(..)
as [Obsolete]
tagged extension method for backwards compatibility. This could be a breaking change for some, so will need to happen as part of a 3.0 major version. PRs are very welcome!
I looked for Set method in IAppCache , and only when looked in implementation, realized that Add is actually Set. It will be good to add explicit Set or AddOrSet method as an alias for current Add.