Improve non-DI usage of built-in feature filters about featuremanagement-dotnet HOT 8 CLOSED

Should we add built-in filters automatically?

For DI usage, built-in filters have already been added automatically. Did you mean we should add them automatically when feature manager is constructed?

Right, for non-DI usage.

It will be nice to have featureManager.FeatureFilters.Add(IFeatureFilter).

Do you mean that it would be better to have the usage look like:

var timeWindowFilter = new TimeWindowFilter();

var featureManager = new FeatureManager(featureDefintionProvider);

featureManager.FeatureFilters.Add(timeWindowFilter);

I wish people couldn't set featureManager.FeatureFilters but add items.

from featuremanagement-dotnet.

@jimmyca15 @rossgrambo @zhenlan What do you think?

from featuremanagement-dotnet.

• It's reasonable for the logger parameter to be optional.
• Should we add built-in filters automatically?
• It will be nice to have featureManager.FeatureFilters.Add(IFeatureFilter).

from featuremanagement-dotnet.

@zhenlan

It's reasonable for the logger parameter to be optional.

Agree.

Should we add built-in filters automatically?

For DI usage, built-in filters have already been added automatically. Did you mean we should add them automatically when feature manager is constructed?

It will be nice to have featureManager.FeatureFilters.Add(IFeatureFilter).

Do you mean that it would be better to have the usage look like:

var timeWindowFilter = new TimeWindowFilter();

var featureManager = new FeatureManager(featureDefintionProvider);

featureManager.FeatureFilters.Add(timeWindowFilter);

from featuremanagement-dotnet.

After discussed with Jimmy, we thought we should keep the current usage:

var featureManager = new FeatureManager(featureDefintionProvider)
{
    FeatureFilters = new List<IFeatureFilterMetadata>() { targetingFilter, timeWindowFilter }
};

Currently, FeatureFilters is a public property with the init accessor. The reason of this design is the idea that we want feature filters to be static after FeatureManager is constructed.

var featureManager = new FeatureManager(featureDefintionProvider);

featureManager.FeatureFilters.Add(timeWindowFilter);

This usage will break this idea.

@jimmyca15 @zhenlan

from featuremanagement-dotnet.

I understand featureManager.FeatureFilters cannot be set to another List object once it's initialized, but can't people call Add on the existing list object anyway?

from featuremanagement-dotnet.

@zhenlan

but can't people call Add on the existing list object anyway

FeatureManager.FeatureFilters is an IEnumerable, no filters can be added. By design so that the feature manager is constructed as intended from the onset.

from featuremanagement-dotnet.

I see. Thanks. I don't have any better ideas, but kind of a bummer users have to know IFeatureFilterMetadata even just to use built-in filters.

from featuremanagement-dotnet.