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

Add expert filter feature #71

Merged
merged 13 commits into from
Nov 2, 2023
Merged

Add expert filter feature #71

merged 13 commits into from
Nov 2, 2023

Conversation

antoinebhs
Copy link
Contributor

No description provided.


@Override
public boolean evaluateRule(String identifiableValue) {
return false;
Copy link
Contributor

@thangqp thangqp Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should implement this method as other single rule? Take the implementation found in ExpertFilterUtils
` public static <I extends Injection> boolean evaluateExpertFilter(AbstractExpertRule filter, Injection injection) {
// As long as there are rules, we go down the tree
if (CombinatorType.AND == (filter.getCombinator())) {
return evaluateAndCombination(filter, injection);
} else if (CombinatorType.OR == filter.getCombinator()) {
return evaluateOrCombination(filter, injection);
} else {
// Evaluate individual filters
return filter.evaluateRule(getFieldValue(filter.getField(), injection));
}
}

private static <I extends Injection<I>> boolean evaluateOrCombination(AbstractExpertRule filter, Injection<I> injection) {
    for (AbstractExpertRule rule : filter.getRules()) {
        // Recursively evaluate the rule
        if (evaluateExpertFilter(rule, injection)) {
            // If any rule is true, the whole combination is true
            return true;
        }
    }
    return false;
}`

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not possible with this model to implement what you suggest. Combinator rules are a special type of rules and should be handled separately. The AbstractExpertRule.evaluateRule() function is meant to evaluate a rule with respect to a value not to a combinator. This applies only to boolean, enum, string or number rules. See for example the signature of evaluateRule() compared with public static <I extends Injection> boolean evaluateExpertFilter(AbstractExpertRule filter, Injection injection) that you suggest moving to CombinatorRule.evaluateRule().
We could implement in CombinatorRule.evaluateRule(String combinatorValue) something like getCombinator().toString().equals(combinatorValue) but I don't think it should be used this way as it's a particular type of rule.

Comment on lines 21 to 53
public static <I extends Injection<I>> boolean evaluateExpertFilter(AbstractExpertRule filter, Injection<I> injection) {
// As long as there are rules, we go down the tree
if (CombinatorType.AND == (filter.getCombinator())) {
return evaluateAndCombination(filter, injection);
} else if (CombinatorType.OR == filter.getCombinator()) {
return evaluateOrCombination(filter, injection);
} else {
// Evaluate individual filters
return filter.evaluateRule(getFieldValue(filter.getField(), injection));
}
}

private static <I extends Injection<I>> boolean evaluateOrCombination(AbstractExpertRule filter, Injection<I> injection) {
for (AbstractExpertRule rule : filter.getRules()) {
// Recursively evaluate the rule
if (evaluateExpertFilter(rule, injection)) {
// If any rule is true, the whole combination is true
return true;
}
}
return false;
}

private static <I extends Injection<I>> boolean evaluateAndCombination(AbstractExpertRule filter, Injection<I> injection) {
for (AbstractExpertRule rule : filter.getRules()) {
// Recursively evaluate the rule
if (!evaluateExpertFilter(rule, injection)) {
// If any rule is false, the whole combination is false
return false;
}
}
return true;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to the implementation of CombinatorExpertRule#evaluateRule

@antoinebhs
Copy link
Contributor Author

I created #75 for the reorganization of files by filter type.

@antoinebhs antoinebhs requested a review from thangqp November 2, 2023 13:09
Copy link

sonarqubecloud bot commented Nov 2, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

89.3% 89.3% Coverage
0.0% 0.0% Duplication

@antoinebhs antoinebhs merged commit a80cb59 into main Nov 2, 2023
4 checks passed
@antoinebhs antoinebhs deleted the expert-filter branch November 2, 2023 16:39
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

Successfully merging this pull request may close these issues.

3 participants