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

CachedVariable for ChildSelectors #29

Open
TravisOnGit opened this issue Aug 20, 2020 · 1 comment
Open

CachedVariable for ChildSelectors #29

TravisOnGit opened this issue Aug 20, 2020 · 1 comment
Labels
enhancement New feature or request

Comments

@TravisOnGit
Copy link
Member

• Add CachedVariable (name pending) property that caches the result of an expression for use in the ChildSelectors.
	○ A common pattern in ChildSelectors is a switch statement, where you calculate some value and visit different children depending on the result. The issue today is this calculation must be done for each child selector. This feature would allow you to calculate the result once and use it directly multiple times. 
	○ Allows you to evaluate an expression and save the result (non-persisted?).
	○ ChildSelectors then have access to that variable to simplify their ShouldSelect statements.
	○ Consider allowing the variable to be a dynamic object. So could be bool, string, object, etc.. This allows users to set as many properties as they wish.
	○ Consider what using a dynamic object would look like. If we have to wrap it every time with (Session.CachedVariable as string), then the usability goes down. In this case, it may be better to just set it as a string. Users could still use it as a bool or object by converting the string if they desire.
	○ Consider if this should be localized to just the current TreeNode's ChildSelector, or if you can access parent's CachedVariables as well.
	○ Consider if this should be tied to ChildSelector at all. Perhaps just adding a way to persist data in the ForgeStateDictionary from the tree would be cool.
		§ Perhaps they both could happen though. This would be a helper property to make the feature intention clear. If we decide to add more functionality later for setting ForgeState, we can still do that.
	○ Considerations:
		§ Persisted vs non-persisted
		§ Useable from ChildSelector only or everywhere?
		§ Save as String vs Object
		§ Cache result vs expression itself. Specify if you want to cache the expression result, or reevaluate.
		§ Consider defining at the tree level.
		§ Add flag to dictate behavior of CachedVariable. Whether or not to reevaluate each time. Could be unclear what the behavior is by default, make sure to set up the right expectations.
	○ Issue with tree structure
		§ How to enforce that people are calling it at the right time? For example, the expression could GetOutput of a TreeNode that hasn't executed yet.
	○ Issue with node structure
		§ If we decide to persist, not having good discoverability on parent nodes persisted data.
	○ Example Old way:
                "ChildSelector": [
                    {
                        "ShouldSelect": "C#|(await Session.GetLastActionResponseAsync()).Status == \"Failure\"",
                        "Child": "SoC_HS_RebootOnly_Failure"
                    },
                    {
                        "ShouldSelect": "C#|(await Session.GetLastActionResponseAsync()).Status == \"Timeout\"",
                        "Child": "SoC_HS_RebootOnly_Timeout"
                    },
                    {
                        "ShouldSelect": "C#|(await Session.GetLastActionResponseAsync()).Status == \"Success\"",
                        "Child": "SoC_HS_RebootOnly_Success"
                    }
	○ Example New way:
                "ChildSelectorVariable": "C#|(await Session.GetLastActionResponseAsync()).Status",
                "ChildSelector": [
                    {
                        "ShouldSelect": "C#|Session.CachedVariable == \"Failure\"",
                        "Child": "SoC_HS_RebootOnly_Failure"
                    },
                    {
                        "ShouldSelect": "C#|Session.CachedVariable == \"Timeout\"",
                        "Child": "SoC_HS_RebootOnly_Timeout"
                    },
                    {
                        "ShouldSelect": "C#|Session.CachedVariable == \"Success\"",
                        "Child": "SoC_HS_RebootOnly_Success"
                }
@TravisOnGit TravisOnGit added the enhancement New feature or request label Aug 20, 2020
@TravisOnGit
Copy link
Member Author

Another idea on top of this. Ability to save yourself an extra Roslyn expression by checking if the CachedVariable equals the ShouldSelect statement.

{
"ShouldSelect": "CachedVariable|"Success"",
"Child": "Result_Success"
},
{
"ShouldSelect": "CachedVariable|"Failure"",
"Child": "Result_Failure"
}

Similar to the built-in "C#|" to execute a Roslyn code snippet, "CachedVariable|" could be used to directly equate the CachedVariable to the property, and return a bool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant