-
Notifications
You must be signed in to change notification settings - Fork 10
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 exhaustive match #26
base: master
Are you sure you want to change the base?
Conversation
Need to add the same option to |
c108f53
to
e8d7d01
Compare
Added a few more enhancements to the idea to provide better error messages, as well as a pessimistic and optimistic version of failure. Thoughts? |
# | ||
# @return [Boolean] | ||
def all_branches_provided? | ||
available_branch_names == @provided_matchers.uniq |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May fail if you provide branches in order other than order of registration
available_branch_names == @provided_matchers.uniq | |
available_branch_names.sort == @provided_matchers.uniq.sort |
or you can store names as an instance of Set
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to integrate latest changes into my PR, and got this error:
Qo::Exceptions::ExhaustiveMatchMissingBranches:
Exhaustive match required: pattern does not specify all branches.
Expected Branches: right, left
Given Branches: left, right
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may jump back to implementing this with sets, silly mistake on my part.
# | ||
# @author baweaver | ||
# @since 0.99.1 | ||
class ExhaustiveMatchMissingBranches < StandardError |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest having common ancestor for all Qo errors and, probably for exhaustive match errors. So end users will be able to catch errors with desired granularity. For example Qo::Error, Qo, MatchError, ExhaustiveMatchNotMet etc.
Adds the idea of an exhaustive match to the API that will fail if a match has not been found as a response to #25