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

Interpretation of Type on ExecuteOrderRule is confusing and used in two different ways #48

Closed
roryprimrose opened this issue Aug 6, 2017 · 0 comments

Comments

@roryprimrose
Copy link
Owner

roryprimrose commented Aug 6, 2017

The DefaultBuildStrategyCompiler uses type on ExecuteOrderRules to identify the ordering of evaluation by property type. The DefaultExecuteStrategy uses the rules against the property type so this works as intended.

The expression extension methods however use the owning type of the property to register a rule. This work syntactically but is very broken when the rule is used by DefaultExecuteStrategy.

For example:


    public static partial class Create
    {
        public static T Model<T>()
        {
            var compiler = ModelBuilder.Model.BuildStrategy.Clone();

            compiler.AddExecuteOrderRule<Skill>(x => x.YearStarted, 4000);
            compiler.AddExecuteOrderRule<Skill>(x => x.YearLastUsed, 3000);

            compiler.AddValueGenerator<YearInPastValueGenerator>();

            var builder = compiler.Compile();

            return builder.Create<T>();
        }
    }

This should add order rules such that YearStarted is evaluated first and YearLastUsed is evaluated afterwards. The bug is that AddExecuteOrderRule creates an order rule against typeof(Skill) instead of typeof(int?) which is the property type determined from the expression.

Both scenarios are valid cases. There should be the option to target the ExecuteOrderRule against a property type as well as owning type of the property. This would cater for being able to run strings after enums for example, while also using expressions to target specific properties.

@roryprimrose roryprimrose changed the title AddExecuteOrderRule<T> registers order against T instead of expression property type Interpretation of Type on ExecuteOrderRule is confusing and used in two different ways Aug 8, 2017
roryprimrose added a commit that referenced this issue Aug 10, 2017
* Updated ExecuteOrderRule internals to support an declaring type.
* Added overloads to ExecuteOrderRule to work with both declaringType and propertyType.
* Updated DefaultExecuteStrategy to provide declaring type as well as property type to ExecutionOrderRule.
* Updated BuildStrategyCompilerExtensions to correctly create ExecuteOrderRule from an expression.

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

No branches or pull requests

1 participant