Skip to content

Added Condition shortcut suggestion#46

Merged
bjdmeest merged 5 commits intomainfrom
feature/condition
Oct 9, 2024
Merged

Added Condition shortcut suggestion#46
bjdmeest merged 5 commits intomainfrom
feature/condition

Conversation

@bjdmeest
Copy link
Member

@bjdmeest bjdmeest commented Aug 2, 2024

No description provided.

@bjdmeest bjdmeest requested review from dachafra and pmaria August 2, 2024 09:31
@bjdmeest bjdmeest self-assigned this Aug 2, 2024
@bjdmeest
Copy link
Member Author

bjdmeest commented Aug 2, 2024

Current PR needs some more details, but I prefer to have you have a look at the overall idea first before I spend more time

Copy link

@pmaria pmaria left a comment

Choose a reason for hiding this comment

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

So a rml:condition is a new property of an ExpressionMap, that if it evaluates to true causes the expression map to produce a result, but if false causes the expression map to return an empty list.

That in principle sounds good to me.

However, if we use a function execution as value of a condition, one thing that might now cause some problems in the current design of fnml is the separation of rml:functionExecution and rml:return on rml:ExpressionMap. Since rml:condition is also placed on the same level, this means that we cannot use rml:return for conditions. That seems like a design issue to me.

Perhaps we can solve this by making the value of a condition an expression map as well?

Some specific comments inline

@bjdmeest
Copy link
Member Author

However, if we use a function execution as value of a condition, one thing that might now cause some problems in the current design of fnml is the separation of rml:functionExecution and rml:return on rml:ExpressionMap. Since rml:condition is also placed on the same level, this means that we cannot use rml:return for conditions. That seems like a design issue to me.

Perhaps we can solve this by making the value of a condition an expression map as well?

Yes. I agree and changed accordingly.

@bjdmeest bjdmeest merged commit 9400519 into main Oct 9, 2024
@bjdmeest bjdmeest deleted the feature/condition branch August 6, 2025 14:54
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