ZiggyCreatures / FusionCache

FusionCache is an easy to use, fast and robust hybrid cache with advanced resiliency features.
MIT License
1.92k stars 97 forks source link

ZiggyCreatures.FusionCache.Abstractions? #43

Closed adambajguz closed 2 years ago

adambajguz commented 2 years ago

Hi @jodydonetti, have you ever considered extracting interfaces and common classes to separate library like ZiggyCreatures.FusionCache.Abstractions? This may allow plugins and higher app layers not to depend on main package.

jodydonetti commented 2 years ago

Hi @adambajguz , I observed the pattern of separating core interfaces and common classes in an "abstractions" package in the wild, but all the times it was because the main package had a lot of other sub-dependencies.

In the case of FusionCache though, the main package only depends on 2 other packages which are Microsoft.Extensions.Caching.Memory (for the memory cache) and System.Threading.Tasks.Extensions (for ValueTasks): both of those dependencies would be there in the "abstractions" package nonetheless, since the related types are used directly in the public API surface area, so I don't know what would be gained by this split (in this specific case at least).

Can you elaborate more about why you think this would useful? I'm interested in understanding more.

Anyway I'm certainly open to the change if there's a real advantage.

Thanks!

adambajguz commented 2 years ago

Hi @jodydonetti, I think the main idea of ".Abstractions" package is to provide contracts/API (that are considered most stable, independent, and universal). The case, where we need ".Abstractions" package to provide multiple different implementations of IFusionCache, is probably not valid here (unless you would like to introduce ICache and build a new caching API standard).

However ".Abstractions" may result in easier/better compatibility for plugins etc. It is less likely for ".Abstractions" to have breaking changes in existing API, so even updating major versions should be easier, e.g. old plugin is guaranteed/more likely to work properly with newer version.

Also having ".Abstractions" package means concrete implementation and usable interfaces are separated - this can be used in complex applicaitons, where cache is configured in one assembly and is used in another, explicilty dissalowing access to concrete implementation classes (this is an example of a very pedantic and safe approaoch where we want to have an application layer consisting of only very very stable external interfaces).

Finally, in my opinion ".Abstractions" package helps to understand the library code as it is much easier to identifiy important files and their default implementation.

adambajguz commented 2 years ago

Almost forgotten to write: I think ".Abstractions" do not require Microsoft.Extensions.Caching.Memory but rather 'Microsoft.Extensions.Caching.Abstractions' - this however needs further verification.

jodydonetti commented 2 years ago

In this approach would you supposedly also use an "Abstractions" namespace (on top of the separate packages) or would you keep the same namespace but only split packages?

I've seen both approaches and I'd like to start gathering the community's position on this while I think about it.

adambajguz commented 2 years ago

I always use namespaces without ".Abstractions" (by adding RootNamespace to csproj). As far as I know this is what Microsoft does in all packages. Also it seems inconvenient to have ".Abstractions" added everywhere (at least for me), and I don't see any benefits of ".Abstractions" in namespaces.

jodydonetti commented 2 years ago

I'm moving this to the recently opened Discussions board.

In case this will be approved to become a feature a separate issue for the design & development will be created.