Add streamlined way for building conditions

[jdromeiro]

Description

Introduces a new way of more fluently building and adding rule conditions:

• for a value condition

 .WithCondition(rc => rc.Value(ConditionType.ClientType, Operators.Equal, "Premium"))

// or (for a bit more sugar)
 .WithCondition(ConditionType.ClientType, Operators.Equal, "Premium")

• for a composed condition

.WithCondition(rc => rc
    .And(a => a
        .Value(ConditionType.ClientType, Operators.Equal, "Queen")
        .Value(ConditionType.Country, Operators.In, new[] { "Portugal", "Spain" })
        .Or(o => o
            .Value(ConditionType.Country, Operators.Equal, "Brazil")
            .Value(ConditionType.Country, Operators.NotEqual, "United States")
        )
    ))

It also adds a new contructor for the Condition class.

Closes #138 .

Change checklist

• Code follows the code rules guidelines of this project
• Commit messages follow the commit rules of this project
• I have self-reviewed my changes before submitting this pull request
• I have covered new/changed code with new tests and/or adjusted existent ones
• I have made changes necessary to update the documentation accordingly

Please also check the I want to contribute guidelines and make sure you have done accordingly.

Disclaimer

By sending us your contributions, you are agreeing that your contribution is made subject to the terms of our Contributor Ownership Statement

[jdromeiro]

@luispfgarces can you please let me know if you agree with the modifications before changing all the tests and samples to use this streamlined way?

[luispfgarces]

Hi @jdromeiro,

I like your approach, but if you'll allow me, let me suggest taking this even further. I have created a small graph representing the calls to create a rule's conditions.

Basically, it only changes the way we start defining conditions (the root condition) and the composed nodes (and/or), the rest is just naming. Let's get one example:

• For the current way of defining the rule.

RuleBuilder.NewRule<ContentTypes, ConditionTypes>()
    .WithName("Benchmark 2 - Bohemian Rapsody")
    .WithDateBegin(DateTime.Parse("2000-01-01"))
    .WithContent(ContentTypes.Songs, "Bohemian Rapsody")
    .WithCondition(x =>
    {
        return x.AsComposed()
            .WithLogicalOperator(LogicalOperators.And)
            .AddCondition(c =>
                c.AsValued(ConditionTypes.Artist)
                    .OfDataType<string>()
                    .WithComparisonOperator(Operators.Equal)
                    .SetOperand("Queen")
                    .Build())
            .AddCondition(c =>
                c.AsValued(ConditionTypes.Lyrics)
                    .OfDataType<string>()
                    .WithComparisonOperator(Operators.Contains)
                    .SetOperand("real life")
                    .Build())
            .AddCondition(c =>
                c.AsValued(ConditionTypes.ReleaseYear)
                    .OfDataType<int>()
                    .WithComparisonOperator(Operators.GreaterThanOrEqual)
                    .SetOperand(1973)
                    .Build())
            .AddCondition(c =>
                c.AsValued(ConditionTypes.ReleaseYear)
                    .OfDataType<int>()
                    .WithComparisonOperator(Operators.LesserThanOrEqual)
                    .SetOperand(1977)
                    .Build())
            .Build();
    })
    .Build();

• Now with the mash-up of our suggestions.

RuleBuilder.NewRule<ContentTypes, ConditionTypes>()
    .WithName("Benchmark 2 - Bohemian Rapsody")
    .WithDateBegin(DateTime.Parse("2000-01-01"))
    .WithContent(ContentTypes.Songs, "Bohemian Rapsody")
    .WithCondition(x => 
        x.And(c => c.Value(ConditionTypes.Artist, Operators.Equal, "Queen")
            .Value(ConditionTypes.Lyrics, Operators.Contains, "real life")
            .Value(ConditionTypes.ReleaseYear, Operators.GreaterThanOrEqual, 1973)
            .Value(ConditionTypes.ReleaseYear, Operators.LesserThanOrEqual, 1977)
        )
    )
    .Build();

What are your thoughts on this? Give it a try first if you want to find any flaws in my suggestion, as I have not tested it 🙂

[jdromeiro]

Hello @luispfgarces ! Thanks for your suggestion, I agree it improves the solution. 👌🏼
Considering your snippet, I've managed to achieve the following behaviour:

• for cases with a single rule

.WithCondition(BasicConditionType.ClientType, Operators.Equal, "Premium")

• for cases with a composed rule

.WithConditions(LogicalOperators.And, x => x
                  .Value(BasicConditionType.ClientType, Operators.Equal, "Queen")
                  .Value(BasicConditionType.Country, Operators.In, new[] { "Portugal", "Spain" })
                  .Or(y => y
                      .Value(BasicConditionType.Country, Operators.Equal, "Brazil")
                      .Value(BasicConditionType.Country, Operators.NotEqual, "United States")
                  )
               )

Given that the method in the RuleBuilder (or RuleBuilderExtension) should only accept one composedCondition, didn't manage to make it exactly how you had describe it (because we should only have one LogicalOperator, which in the proposal is being passed as parameter).

I've pushed the new code. Please let me know what you think.

[luispfgarces]

Hi @jdromeiro,

Thank you for taking my input into consideration 👌 if you are interested in resolving that last bit you couldn't resolve, I may have a possible solution for you. The thing is, with the current builders you can't ensure a single "root condition" because on fluent APIs you have to design your interfaces and classes according to the possible state machine to be accomplished. That's why I told you once that fluent APIs require planning representing a state machine on a graph using a tool you'd like (e.g. draw.io) and lots of trial and error sometimes 😄

Here goes my even further improved suggestion:

• Create interfaces IRootConditionNodeBuilder<TConditionType> and IConfiguredRootConditionNodeBuilder<TConditionType>.

public interface IRootConditionNodeBuilder<TConditionType>
{
    IConfiguredRootConditionNodeBuilder<TConditionType> And(Func<IComposedConditionNodeBuilder<TConditionType>, IComposedConditionNodeBuilder<TConditionType>> conditionFunc);
    IConfiguredRootConditionNodeBuilder<TConditionType> Or(Func<IComposedConditionNodeBuilder<TConditionType>, IComposedConditionNodeBuilder<TConditionType>> conditionFunc);
    IConfiguredRootConditionNodeBuilder<TConditionType> Value<TDataType>(TConditionType conditionType, Operators condOperator, TDataType operand);
}

public interface IConfiguredRootConditionNodeBuilder<TConditionType>
{
    IConditionNode<TConditionType> GetRootCondition();
}

• Create class RootConditionNodeBuilder<TConditionType> that implements both interfaces.

internal class RootConditionNodeBuilder<TConditionType> : IRootConditionNodeBuilder<TConditionType>, IConfiguredRootConditionNodeBuilder<TConditionType>
{
    private IConditionNode<TConditionType> rootCondition;

    public IConfiguredRootConditionNodeBuilder<TConditionType> And(Func<IComposedConditionNodeBuilder<TConditionType>, IComposedConditionNodeBuilder<TConditionType>> conditionFunc)
    {
        // Do your already implemented logic. Assign result to rootCondition.

        return this;
    }

    public IConfiguredRootConditionNodeBuilder<TConditionType> Or(Func<IComposedConditionNodeBuilder<TConditionType>, IComposedConditionNodeBuilder<TConditionType>> conditionFunc)
    {
        // Do your already implemented logic. Assign result to rootCondition.

        return this;
    }
    public IConfiguredRootConditionNodeBuilder<TConditionType> Value<TDataType>(TConditionType conditionType, Operators condOperator, TDataType operand)
    {
        // Do your already implemented logic. Assign result to rootCondition.

        return this;
    }

    public IConditionNode<TConditionType> GetRootCondition()
        => this.rootCondition;
}

• This way you only need one extension method on the RuleBuilderExtensions.

public static IRuleBuilder<TContentType, TConditionType> WithCondition<TContentType, TConditionType>(this IRuleBuilder<TContentType, TConditionType> ruleBuilder, Func<IRootConditionNodeBuilder<TConditionType>, IConfiguredRootConditionNodeBuilder<TConditionType>> conditionFunc)

With this, you could accomplish the suggestion I gave you, which has the advantage of defining conditions always the same way (from the consumer's perspective). Maybe this is a bit of nitpicking on my part, but it makes sense to me to offer the best experience possible to the developers that use the fluent API 😄 what do you think?

[jdromeiro]

@luispfgarces , thank you for your suggestions! 👌🏼

I've taken your snippets and adjusted them a bit. I've also created a new entity IChildComposedConditionNodeBuilder (for which the logic could be kept as the IComposedConditionNodeBuilder) because I thought would be easier to separate the old way from the new way of creating the conditions.

If you agree, we can set the old methods as obsolete, and in the future remove them or make them internal.

Let me know of your thoughts regarding the solution as it is now.

[luispfgarces]

Hi @jdromeiro,

I think you have nailed it this time 💪 I agree with this approach, I'm pro making the current conditions builder API obsolete and using the new improved API you proposed. It would be best if you marked the value condition nodes builder API obsolete also, to state to the consumers "this is the new way to build all the condition nodes for a rule, the old one will be totally deprecated".

I left some comments with small improvements to be made. I will leave my approval to be done later when you have fixed everything (and also leave you the room to refactor anything you see fit if you need to).

[jdromeiro]

Sorry @luispfgarces , decided to also replace all calls to WithContentContainer(). One more 👍🏼 please 😅