Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Set Enabled in StaticHelper to afect all instances #129

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

marioanogueira
Copy link

Background for this PR. Having a few experiments running without using the Static Helper, was trying to disable all experiments at once, by using the Enabled method. Turns out it wasn't possible.

To achieve this Had to create a implementation of Scientist and override the IsEnabledAsync.

This PR aims to allow the set of the Enabled func globally, for experiments using the helper and for the implementations.
The IsEnabledAsync remains virtual so that we can still override the enabled behaviour for a specific one

@M-Zuber
Copy link
Contributor

M-Zuber commented Jul 7, 2019

I am not sure how the changes accomplish the stated goal, could you maybe provide some code samples to show the difference in use between the current code, and the code you are proposing?

@marioanogueira
Copy link
Author

Sure, so right now we have two ways of starting an Experiment, either by using the Static helper like
Experiment<bool>(ExperimentName, experiment => { experiment.Use(() => true); experiment.Try(() => false); });
or by creating a new Scientist object like this
var scientist = new Scientist(resultPublisher); scientist.Experiment<bool>("", experiment => { experiment.Use(() => true); experiment.Try(() => false); });
Given this, if we have multiple experiments on both formats and we want to use the Enabled function to disable or enable all of the experiments the current implementation doesn't allow it. The current Enabled function only affects experiments created using the static helper.

My changes allow the Enabled to affect all of the esperiments regardless if they are being create using the static helper or the instance

@M-Zuber
Copy link
Contributor

M-Zuber commented Jul 11, 2019

One change that could be helpful in evaluating this would be to revert the naming change of _sharedScientist to SharedScientist

Do you think you could write a test that will only pass with this change?

@marioanogueira
Copy link
Author

Yeah sure, I'll write the test and update the PR.

Will revert the name change also

@M-Zuber
Copy link
Contributor

M-Zuber commented Jul 11, 2019

Thank you!
(also, you might to keep this in mind: #108 (comment))

@marioanogueira
Copy link
Author

marioanogueira commented Jul 11, 2019

I do have a few more ideas to make it more IoC friendly, but I wanna do them in a different PR.
My changes won't affect the current friendliness of IoC/DI, in fact it will improve since it will allow to disable experiments that are being created by a IoC container

@M-Zuber M-Zuber mentioned this pull request Jul 11, 2019
@marioanogueira
Copy link
Author

@M-Zuber did you had time to review the latest changes?

M-Zuber
M-Zuber previously approved these changes Jul 22, 2021
Mario Nogueira and others added 3 commits July 22, 2021 08:56
Allows to define the Enabled function without having to create a
special implementation for when not using the helper.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants