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

Setting default comparator for namespace_map to be the expected behaviour. #8

Merged
merged 1 commit into from
Jul 13, 2023

Conversation

GhostofCookie
Copy link
Contributor

Very interesting stuff this comparator, learning new stuff all the time. The test looks like a lot has changed, but hilariously, it was working exactly as intended. The changes to the test are just general cleanup, they do NOT change it's behaviour at all.

The big change here is that the default comparator for quicr::namespace_map is now std::greater instead of std::less. The reason for this is simple: My original assumption about what was being found was wrong. The real behaviour:

  • When sorting with std::less, when using a quicr::Name to index it, the shortest match will be returned.
  • When sorting with std::greater, when using a quicr::Name to index it, the longest match will be returned.

For whatever reason, I seemed to have this reversed in my head, and yet still wrote the correct test that showed that exact result. I suppose the way the test was written before was vague and unhelpful. So, the cleanup of the test and some added comments to the type itself should clear up the issue.

Also changed, but not really important: quicr::namespace_map is now a concrete type inheriting from std::map, instead of just being an alias.

@GhostofCookie GhostofCookie merged commit f8c9677 into main Jul 13, 2023
2 checks passed
@GhostofCookie GhostofCookie deleted the fix-namespace-map branch July 13, 2023 16:49
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.

1 participant