Semantic filtering client-side evaluation #109
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Eddykasp
added
enhancement
| export class SemanticFilterTag implements SemanticFilterRule { | ||
| ruleName?: string | ||
| tag: string | ||
| /** Not defining num defaults it to 0. */ |
There was a problem hiding this comment.
When creating such an element here would default to it being undefined and not 0. I guess here you refer to tags from the server will default to 0? Then I would remove the ? here as a value is guaranteed.
Comment on lines
166
to
192
| //// Numeric Connectives //// | ||
|
|
||
| /** | ||
| * A LessThan Connective takes one rule R and evaluates to true | ||
| * iff | ||
| * R.num < correspondingTag.num. | ||
| * @example R.num < correspondingTag.num | ||
| */ | ||
| export class LessThanConnective implements UnaryConnective { | ||
| static NAME = "LESSTHAN" | ||
| name = LessThanConnective.NAME | ||
| operand: SemanticFilterTag | ||
| ruleName?: string | ||
| } | ||
|
|
||
| /** | ||
| * A GreaterThan Connective takes one rule R and evaluates to true | ||
| * iff | ||
| * R.num > correspondingTag.num. | ||
| * @example R.num > correspondingTag.num | ||
| */ | ||
| export class GreaterThanConnective implements UnaryConnective { | ||
| static NAME = "GREATERTHAN" | ||
| name = GreaterThanConnective.NAME | ||
| operand: SemanticFilterTag | ||
| ruleName?: string | ||
| } |
There was a problem hiding this comment.
Default relation definitions usually is using <=, >= and = resp. != instead of < and >
Comment on lines
261
to
278
| case NegationConnective.NAME: | ||
| return !(evaluateRule(unary.operand, tags)); | ||
| case AndConnective.NAME: | ||
| return evaluateRule(binary.leftOperand, tags) | ||
| && evaluateRule(binary.rightOperand, tags); | ||
| case OrConnective.NAME: | ||
| return evaluateRule(binary.leftOperand, tags) | ||
| || evaluateRule(binary.rightOperand, tags); | ||
| case IfThenConnective.NAME: | ||
| return !evaluateRule(binary.leftOperand, tags) | ||
| || evaluateRule(binary.rightOperand, tags); | ||
| case LogicEqualConnective.NAME: | ||
| return evaluateRule(binary.leftOperand, tags) | ||
| === evaluateRule(binary.rightOperand, tags); | ||
| case IfThenElseConnective.NAME: | ||
| return evaluateRule(ternary.firstOperand, tags) | ||
| ? evaluateRule(ternary.secondOperand, tags) | ||
| : evaluateRule(ternary.thirdOperand, tags); |
There was a problem hiding this comment.
I think the evaluation of the directives would be better placed in the namespace of the directive classes in some evaluate function.
There was a problem hiding this comment.
could that not cause problems with the deserialization?
There was a problem hiding this comment.
serialization only uses fields and no methods of classes, I am not quite sure about the other way around. But Actions circumvent this by putting the function into the namespace, not into some class.
There was a problem hiding this comment.
ah ok, I can do it that way
Comment on lines
380
to
442
| export class NumericPlusConnective implements BinaryConnective { | ||
| static NAME = "NUMERICPLUS" | ||
| name = NumericPlusConnective.NAME | ||
| leftOperand: SemanticFilterRule | ||
| rightOperand: SemanticFilterRule | ||
| } | ||
|
|
||
| export namespace NumericPlusConnective { | ||
| export function evaluate(conn: NumericPlusConnective): number { | ||
| return evaluateNumeric(conn.leftOperand) + evaluateNumeric(conn.rightOperand); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * A Numeric Minus Connective takes two numeric operands and evaluates | ||
| * to their difference. | ||
| */ | ||
| export class NumericMinusConnective implements BinaryConnective { | ||
| static NAME = "NUMERICMINUS" | ||
| name = NumericMinusConnective.NAME | ||
| leftOperand: SemanticFilterRule | ||
| rightOperand: SemanticFilterRule | ||
| } | ||
|
|
||
| export namespace NumericMinusConnective { | ||
| export function evaluate(conn: NumericMinusConnective): number { | ||
| return evaluateNumeric(conn.leftOperand) - evaluateNumeric(conn.rightOperand); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * A Numeric Times Connective takes two numeric operands and evaluates | ||
| * to their product. | ||
| */ | ||
| export class NumericTimesConnective implements BinaryConnective { | ||
| static NAME = "NUMERICTIMES" | ||
| name = NumericTimesConnective.NAME | ||
| leftOperand: SemanticFilterRule | ||
| rightOperand: SemanticFilterRule | ||
| } | ||
|
|
||
| export namespace NumericTimesConnective { | ||
| export function evaluate(conn: NumericTimesConnective): number { | ||
| return evaluateNumeric(conn.leftOperand) * evaluateNumeric(conn.rightOperand); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * A Numeric Divides Connective takes two numeric operands and evaluates | ||
| * to their product. | ||
| */ | ||
| export class NumericDividesConnective implements BinaryConnective { | ||
| static NAME = "NUMERICDIVIDES" | ||
| name = NumericDividesConnective.NAME | ||
| leftOperand: SemanticFilterRule | ||
| rightOperand: SemanticFilterRule | ||
| } | ||
|
|
||
| export namespace NumericDividesConnective { | ||
| export function evaluate(conn: NumericDividesConnective): number { | ||
| return evaluateNumeric(conn.leftOperand) / evaluateNumeric(conn.rightOperand); | ||
| } | ||
| } |
There was a problem hiding this comment.
I suggest to rename these to …Addition…, …Subtraction…, …Multiplication…, and …Division…
fight4day
reviewed
Aug 2, 2022
fight4day
reviewed
Aug 2, 2022
| function evaluateNumeric(rule: SemanticFilterRule, tags: Array<SemanticFilterTag>): number { | ||
| // Rule is a Tag | ||
| if ((rule as SemanticFilterTag).tag !== undefined) { | ||
| let nodeTag = tags.find((tag: SemanticFilterTag) => tag.tag === (rule as SemanticFilterTag).tag) |
There was a problem hiding this comment.
Should be const to pass checks
|
superseded by #86 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
kieler/KLighD#137