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

Added _yes get_or_intern methods #43

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

qm3ster
Copy link

@qm3ster qm3ster commented Jul 23, 2023

Adds get_or_intern_yes, try_get_or_intern_yes, get_or_intern_static_yes, try_get_or_intern_static_yes methods that return an additional boolean that answers questions like: "Was this a new, never before seen string?", "Am I the first person to receive this key?", "Should I go compile a Regex or other string derivative and store it in a keyed-alike collection eagerly?", etc.

This allows doing additional things on the first time a string is seen, without hashing twice.

I updated the doctests to actually test the correctness of the bool returned, but didn't otherwise add tests.

I didn't touch the Interner trait because it's not sealed and adding the new methods to it would be a breaking change.
(An honest default implementation would hash twice by calling Resolver::contains_key, not sure if that is something we'd want to publish.)

@qm3ster
Copy link
Author

qm3ster commented Jul 23, 2023

Oh, and obviously _yes is a stupid name and we should call it something else.
And we need to confirm that this is the intuitive order in the returned tuple.

@Kixiron
Copy link
Owner

Kixiron commented Jul 23, 2023

Tuple order is good, I think a name involving insert would be good since that‘s similar to the HashMap api

qm3ster added 2 commits July 23, 2023 23:48
Also let rustfmt sort the dashmap imports 😬
This is a horrendous stability guarantee,
as the string slice storage is now guaranteed.

Also, it should probably be called something else.
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