RimDev.FeatureFlags icon indicating copy to clipboard operation
RimDev.FeatureFlags copied to clipboard

How to register a custom IFeatureProvider

Open unsafecode opened this issue 6 years ago • 6 comments

Looks like FeatureFlagOptions.Provider is read only outside of this lib

unsafecode avatar Apr 11 '19 09:04 unsafecode

Additionally, the implementation of method UseCachedSqlFeatureProvider violates the DI principles, by instanciating the provider directly, and making it complicated to a have real modular architecture.

I would have expected to register an additionaly provider via AddSingleton<IFeatureProvider, MyFeatureprovider>, and let the app choose one via some sort of setting

unsafecode avatar Apr 11 '19 09:04 unsafecode

The internal could be removed from this set;: https://github.com/ritterim/RimDev.FeatureFlags/blob/6a5bd6b6d0078ae570e1b1dd91cbe578a1e719e4/src/RimDev.AspNetCore.FeatureFlags/FeatureFlagOptions.cs#L19

I was thinking that the extension methods would be used instead, but internal does seem to prohibit using a custom provider. Thanks for catching that!


Is there a use case where the IFeatureProvider instance needs to be part of dependency injection that can't be solved without it? Just seeking to understand, since it's possible to do this with branching logic not part of IoC.

kendaleiv avatar Apr 11 '19 12:04 kendaleiv

Honestly, I'd say it's more a matter of philosophy: if you adopt DI, it would be better to leverage it fully.

Additionally, the provider pattern nicely suits DI, as you can even register multiple implementations and then pick one without changing your design.

unsafecode avatar Apr 11 '19 17:04 unsafecode

Added a UseCustomProvider extension method in #27. If we want to change the overall philosophy and results in a breaking change we can look into that with v2.

kendaleiv avatar Apr 18 '19 15:04 kendaleiv

@kendaleiv Thanks for this fix.

IMHO, it would likely be easier not to require an IFeatureProvider instance, and rather have it as a generic parameter in the same method, and then retrieve it via GetRequiredService<T>.

For instance, in the current implementation, how can I pass a registered service in my custom provider instance?

unsafecode avatar Apr 24 '19 14:04 unsafecode

/cc @ettores88 @matteo-daverio

unsafecode avatar Apr 24 '19 14:04 unsafecode