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

Design Decisions #14

Open
wulfland opened this issue Sep 12, 2017 · 3 comments
Open

Design Decisions #14

wulfland opened this issue Sep 12, 2017 · 3 comments

Comments

@wulfland
Copy link

I wanted to discuss two design decisions:

Default Value

I’m not happy with the nullable boolean that is returned from the

Is().Enabled

method. This leaves a lot of complexity to the client. I would prefer a way to add a default value that is returned instead of null. Something like

Is().Enabled.WithDefault(true)

This would leave the old way like it is but also enable default values.

Add “magic string” alternative to class

I normally do not add any logic to my feature class. The class for me is only an overhead that makes me deal with dependencies. If I use the same feature in UI and Data logic this can be a problem. It also is code I have to maintain. I really would like a string alternative:

Feature.ByName("ma-magic-string").Is.Enabled

I’m interested what you think about it.
Maybe I just use the framework other than you do.

@mexx
Copy link
Owner

mexx commented Sep 12, 2017

Default Value

IIRC Feature<Sample>.Is().Enabled has a return type of bool.
If I got you correct on the API in question, then there shouldn't be any additional complexity to the client rather a conditional check for the boolean value. Or do you refer to some other place in the client exposed API?

Magic string

I don't like "magic strings", but I agree the dependency for marking interface to the feature can be cumbersome. I already had in mind to provide an alternative to be able to use any class as a feature.
Your approach is more "radical" to not have a class at all, nevertheless a valid one. However it's currently not supported by the library, and I really don't know if it ever will be. The whole design was based on the presence of a type, representing a feature. See as well the why this library section on the documentation.

One can easily add support for any class as a feature.
Create a generic class, maybe named OfType<T> that implements the IFeature interface along with the corresponding naming convention, which will use the parametrised type to derive the name of the feature. However this limits the discoverability of the features by the means of reflection, as any type in the system might be a feature now. This is why I didn't yet gone that path, but you're free to follow it.

I can imagine that one could provide support for magic strings in a separate library, which can reuse the feature configuration functionality but only use the configured behaviours out of it. However even the Feature.Name requires a type to be passed upon creation. It means some behaviours might not work as expected, as they can rely on the type to identify the feature.

I hope I could answer your questions.

@wulfland
Copy link
Author

Default Value

When I implemented the VstsConfigration I looked how the AwsConnfuguration was implemented. The method is a nulablle bool

public bool? IsEnabled(Feature.Name name)

And there was not method for passing down default values to the configuration store implementation.

Magic strings

Do you use the classes for anything elese then naming the features? Maybe I'm missing some advantage here.

@mexx
Copy link
Owner

mexx commented Sep 14, 2017

Default Value

Ah, I got your confusion.
It's the definition of Feature.Behavior which you refer to.
The bool? is intended there for a behaviour to be able to say: "I can't decide, ask the next guy".
The configuration contains a list of behaviours which will be considered one by one, until a found (not null) decision is returned by the behaviour.

You could have multiple layers of behaviour, what I usually have is the following chain:

  • top most - an in memory behaviour, in which I can override specific features for test purposes
  • some persistent behaviour, app.settings or database
  • optionally more persistent behaviours
  • default value behaviour (anything enabled or disabled)

All of the behaviours except of the default value behaviour only return a proper bool value when the feature is controlled by this behaviour.

Magic strings

Usually the functionality I want to toggle is provided by some class, I tag this type with the interface.
With the marker interface it's easy to collect all features in the software by reflection, as well to find all the places where the feature is used. Just remove the IFeature interface, the compiler shows you all the places by throwing errors at you.

About your former question, if you have to use the same feature in UI and Data logic.
In my experience it's usually only the data logic site that needs to know about the feature, the UI should handle both cases, whether the feature is enabled or not anyway, based on the data returned by the server. What I mean by it is that the client doesn't have to know about the feature configuration but only about the state of the feature which was used on the server in the data logic, and usually this is anyway somehow identifiable in the returned data. If not, provide a separate property which controls the client, but as already mentioned, it would only contain the state on the feature that was used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants