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

Make get/get_mut/remove in HashMap more flexible with Borrow trait #12

Closed
choznerol opened this issue Sep 25, 2018 · 2 comments · Fixed by #13
Closed

Make get/get_mut/remove in HashMap more flexible with Borrow trait #12

choznerol opened this issue Sep 25, 2018 · 2 comments · Fixed by #13
Assignees

Comments

@choznerol
Copy link
Contributor

choznerol commented Sep 25, 2018

I noticed the TODO message along with the helpful hints in the HashMap implementation:

/// Gets a reference to the value under the specified key.
///
/// TODO: To treat owned and borrowed values in equivalent ways as other
/// collections in std do, we should use `Borrow` trait to abstract over
/// the type of key to hash. This concept can also applied for `get_mut`
/// `remove`, and other operations that constrain by the type system.
///
/// Some useful resources:
///
/// - [Trait std::borrow:Borrow][1]
/// - [TRPL 1st edition: Borrow and AsRef][2]
///
/// # Complexity
///
/// Constant (amortized).
///
/// [1]: https://doc.rust-lang.org/stable/std/borrow/trait.Borrow.html
/// [2]: https://doc.rust-lang.org/stable/book/first-edition/borrow-and-asref.html
pub fn get(&self, key: &K) -> Option<&V> {
let index = self.make_hash(key);
self.buckets.get(index).and_then(|bucket|
bucket.iter()
.find(|(k, _)| *k == *key)
.map(|(_, v)| v)
)
}

if it's not currently under development, I would love to give it a try!

@choznerol choznerol self-assigned this Sep 25, 2018
@choznerol choznerol changed the title Make get/get_mut/remove of HashMap more flexible with Borrow trait [WIP] Make get/get_mut/remove of HashMap more flexible with Borrow trait Sep 25, 2018
@choznerol choznerol changed the title [WIP] Make get/get_mut/remove of HashMap more flexible with Borrow trait [WIP] Make get/get_mut/remove in HashMap more flexible with Borrow trait Sep 25, 2018
@choznerol
Copy link
Contributor Author

The test in HashMap are all using this type Map ... in which K and V are set;

type Map<'a> = HashMap<&'a str, &'a str>;

while in SinglyLinkedList, tests define the generic type one by one

let mut l = SinglyLinkedList::<i32>::new();

by completing this TODO, somewhere I'll need to add, for example HashMap<String, String>. Should I just follow the style in SinglyLinkList and update the tests in HashMap, or maybe there are some preferred testing style ?

@weihanglo
Copy link
Owner

@choznerol

Yeah, you're welcome to take the borrow trait task.

As for the testing style, my preference is no preference. Anything making code more clearer is acceptable. If there is some room to improve, just go for it!

@choznerol choznerol changed the title [WIP] Make get/get_mut/remove in HashMap more flexible with Borrow trait Make get/get_mut/remove in HashMap more flexible with Borrow trait Sep 30, 2018
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 a pull request may close this issue.

2 participants