-
-
Notifications
You must be signed in to change notification settings - Fork 56
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
Router validation #637
base: main
Are you sure you want to change the base?
Router validation #637
Conversation
return true | ||
} | ||
guard trieValues.count > 1 else { return } | ||
for index in 1..<trieValues.count { |
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 think this is computationally much heavier than it needs to be, though it will catch more routes that overlap.
Let's break down what we want to validate (and why) in a discussion somewhere.
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 was intending to catch any route that would never be triggered because a higher priority route was triggered instead.
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.
Optimised this. It now does the validation at the trie node level which is considerably faster
Add
Router.validate()
to check whether any route paths clash.Unfortunately cannot add this in
buildResponder
as it is a throwing function.