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

Add get_mut_or_insert #151

Merged
merged 3 commits into from
Sep 4, 2022
Merged

Conversation

Yosi-Hezi
Copy link
Contributor

This PR adds the get_mut_or_insert method, which is based on get_or_insert but returns a mutable reference instead of an immutable reference.
Relevant open issue: #30

@Yosi-Hezi
Copy link
Contributor Author

There is a version of get_mut_or_insert that is based on #150 in case it will be accepted before this one.
I pushed this PR as well because it doesn't break the API, and I could really use the get_mut_or_insert functionality in my own code.

@Yosi-Hezi
Copy link
Contributor Author

Maybe a better name would be get_or_insert_mut?

@jeromefroe
Copy link
Owner

@Yosi-Hezi do you want to rebase onto master now that #150 has been merged? My personal inclination would be for get_or_insert_mut as well :)

@Yosi-Hezi
Copy link
Contributor Author

@jeromefroe rebased

src/lib.rs Outdated
/// assert_eq!(cache.get_or_insert_mut(3, ||"f"), &mut "f");
/// assert_eq!(cache.get_or_insert_mut(3, ||"e"), &mut "f");
/// ```
pub fn get_or_insert_mut<'a, F>(&mut self, k: K, f: F) -> &'a mut V
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub fn get_or_insert_mut<'a, F>(&mut self, k: K, f: F) -> &'a mut V
pub fn get_or_insert_mut<'a, F>(&'a mut self, k: K, f: F) -> &'a mut V

#153

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matklad fixed.

@jeromefroe
Copy link
Owner

Thanks for the PR @Yosi-Hezi!

@jeromefroe jeromefroe merged commit ff6c80e into jeromefroe:master Sep 4, 2022
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.

3 participants