weihanglo/rust-algorithm-club

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

choznerol opened this issue · 2 comments

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!

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 ?

@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!