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

Introduce part operation. #37

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

hwo411
Copy link
Contributor

@hwo411 hwo411 commented Apr 4, 2018

  • Valid when at least one rule is valid
  • Returns AST with all errors in case no rules are valid
  • Add visit_set and visit_part to Result

One of the changes I mentioned in:
https://discourse.dry-rb.org/t/extensions-for-dry-logic-and-dry-validations-to-support-open-api-schemas/525

I have 2 questions about the implementation:

  1. Is name part good?
  2. Is it enough to return Result::SUCCESS or should I put successful or failed rules into successful result?

* Valid when at least one rule is valid
* Returns AST with all errors in case no rules are valid
* Add visit_set and visit_part to Result
@solnic
Copy link
Member

solnic commented Apr 4, 2018

I'd say Any would be a better name. WDYT?

@hwo411
Copy link
Contributor Author

hwo411 commented Apr 4, 2018

@solnic Initially I was thinking about Any, but there exists Each operation which operates on array of items and Set operation, which operates on array of rules. Any as name is closer to Each than to Set and that's why I thought it should have different name than Any. Also the same is true for ruby Enumerable, which has both each and any methods.

@hwo411
Copy link
Contributor Author

hwo411 commented Apr 9, 2018

@solnic Just after some thinking, we can imagine 2 different groups of operations:

  1. Applying a rule to multiple items, Each already exists, I'd expect also Any. and some operation either to verify that rule returns true from a to b items(or just 1). None can be a shortcut to Not(Any(...)) or a separate rule, also there can be All as alias to Each. Not sure if a rule or shortcut for Not(Each(...)) is required.
  2. Applying multiple rules to a single item(it can also be an array, Set already exists, Part is in this PR, I'd expect also expect a rule from a to b rules are true(or just 1). Not sure about negation, it can be exposed via Not(Part(...)) and Not(Set(...)), but we can also provide shortcut or rule.

By shortcut I mean applying 2 rules via 1 method/class without creating new kind of operation and new type of ast.

WDYT?

@hwo411 hwo411 requested a review from solnic as a code owner May 21, 2021 13:06
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.

2 participants