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

Select the longest matching key in matchMap #179

Merged
merged 2 commits into from Jun 21, 2017

Conversation

Projects
None yet
2 participants
@barnardb
Contributor

barnardb commented Mar 3, 2017

Fixes #136

By checking keys from longest to shortest, we ensure that we always
select the longest matching key.

This fix requires sorting the keys each time the rule is invoked. If
#115 gets implemented, this could be improved to make use of it instead.

Select the longest matching key in matchMap
Fixes #136

By checking keys from longest to shortest, we ensure that we always
select the longest matching key.

This fix requires sorting the keys each time the rule is invoked. If
#115 gets implemented, this could be improved to make use of it instead.
@barnardb

This comment has been minimized.

Show comment
Hide comment
@barnardb

barnardb Mar 3, 2017

Contributor

Sorry, I wasn't thinking straight in writing that final comment. The map value isn't known until runtime, so of course the optimisation desired in #115 would be unhelpful.

Contributor

barnardb commented Mar 3, 2017

Sorry, I wasn't thinking straight in writing that final comment. The map value isn't known until runtime, so of course the optimisation desired in #115 would be unhelpful.

Select longest matching map key more efficiently
Only sort the keys until we find the one we're interested in, as suggested in
#179 (review)

@sirthias sirthias merged commit d7ad77a into sirthias:release-2.1 Jun 21, 2017

@sirthias

This comment has been minimized.

Show comment
Hide comment
@sirthias

sirthias Jun 21, 2017

Owner

Thank you, Ben, and sorry for taking so long to get this merged!

Owner

sirthias commented Jun 21, 2017

Thank you, Ben, and sorry for taking so long to get this merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment