Skip to content

Implement HashMap.find_with_or_insert_with() #14196

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
May 16, 2014
Merged

Implement HashMap.find_with_or_insert_with() #14196

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
460b552
Reinstate HashMap.mangle().
chris-morgan May 14, 2014
acdce63
Tidy up the HashMap.mangle example.
chris-morgan May 15, 2014
73dc1e0
Rename HashMap.mangle to find_with_or_insert_with.
chris-morgan May 15, 2014
5b13ddb
Use assertions in find_with_or_insert_with example
chris-morgan May 15, 2014
ff98afe
Work around parse error caused by #14240.
chris-morgan May 16, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 57 additions & 21 deletions src/libcollections/hashmap.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1239,31 +1239,14 @@ impl<K: TotalEq + Hash<S>, V, S, H: Hasher<S>> HashMap<K, V, H> {
/// Return the value corresponding to the key in the map, or insert
/// and return the value if it doesn't exist.
pub fn find_or_insert<'a>(&'a mut self, k: K, v: V) -> &'a mut V {
let hash = self.make_hash(&k);
match self.search_hashed(&hash, &k) {
Some(idx) => {
let (_, v_ref) = self.table.read_mut(&idx);
v_ref
},
None => self.insert_hashed(hash, k, v)
}
self.mangle(k, v, |_k, a| a, |_k, _v, _a| ())
}

/// Return the value corresponding to the key in the map, or create,
/// insert, and return a new value if it doesn't exist.
pub fn find_or_insert_with<'a>(&'a mut self, k: K, f: |&K| -> V)
-> &'a mut V {
let hash = self.make_hash(&k);
match self.search_hashed(&hash, &k) {
Some(idx) => {
let (_, v_ref) = self.table.read_mut(&idx);
v_ref
},
None => {
let v = f(&k);
self.insert_hashed(hash, k, v)
}
}
self.mangle(k, (), |k, _a| f(k), |_k, _v, _a| ())
}

/// Insert a key-value pair into the map if the key is not already present.
Expand All @@ -1275,12 +1258,65 @@ impl<K: TotalEq + Hash<S>, V, S, H: Hasher<S>> HashMap<K, V, H> {
v: V,
f: |&K, &mut V|)
-> &'a mut V {
self.mangle(k, v, |_k, a| a, |k, v, _a| f(k, v))
}

/// Modify and return the value corresponding to the key in the map, or
/// insert and return a new value if it doesn't exist.
///
/// This method allows for all insertion behaviours of a hashmap;
/// see methods like `insert`, `find_or_insert` and
/// `insert_or_update_with` for less general and more friendly
/// variations of this.
///
/// # Example
///
/// ```rust
/// use collections::HashMap;
///
/// // map some strings to vectors of strings
/// let mut map = HashMap::<StrBuf, Vec<StrBuf>>::new();
/// map.insert(StrBuf::from_str("a key"), vec![StrBuf::from_str("value")]);
/// map.insert(StrBuf::from_str("z key"), vec![StrBuf::from_str("value")]);
Copy link
Member

Choose a reason for hiding this comment

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

These might be neater using .to_strbuf() (if not just using &str as alex suggests).

///
/// let new = vec![StrBuf::from_str("a key"),
/// StrBuf::from_str("b key"),
/// StrBuf::from_str("z key")];
/// for k in new.move_iter() {
/// map.mangle(k, StrBuf::from_str("new value"),
/// // if the key doesn't exist in the map yet, add it in
/// // the obvious way.
/// |_k, v| vec![v],
/// // if the key does exist either prepend or append this
/// // new value based on the first letter of the key.
/// |key, already, new| {
/// if key.as_slice().starts_with("z") {
/// already.unshift(new);
/// } else {
/// already.push(new);
/// }
/// });
/// }
Copy link
Member

Choose a reason for hiding this comment

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

This example may be clearer without all the StrBuf, if they're replaced with just &str, does it end up working out?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see how it would. Prepending/appending the strings would have to produce a StrBuf as a result (as it must allocate), so the value type can't be &str.

Copy link
Member

Choose a reason for hiding this comment

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

I believe that already has type &mut Vec<StrBuf>, appending/unshifting on a vector, not a string.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah hah, you're right. I didn't read carefully enough.

///
/// for (k, v) in map.iter() {
/// println!("{} -> {:?}", *k, *v);
Copy link
Member

Choose a reason for hiding this comment

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

We've been generally avoiding :?, does {} not suffice here?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is just the example that existed in 0.9 with the fixes that were necessary to get it to compile. Seeing as there appears to be acceptance of reintroducing this method, I’ll improve the example now.

/// }
Copy link
Contributor

Choose a reason for hiding this comment

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

This would be a little clearer if you asserted equality instead of printing, because I can't see what it prints without actually running the code locally. Instead you could use something like

assert_eq!(map.len(), 3);
assert_eq!(map.get("a key"), vec!["value", "new value"]);
asesrt_eq!(map.get("b key"), vec!["value", "new value"]);
assert_eq!(map.get("z key"), vec!["new value", "value"]);

/// ```
pub fn mangle<'a, A>(&'a mut self,
k: K,
a: A,
not_found: |&K, A| -> V,
found: |&K, &mut V, A|)
-> &'a mut V {
let hash = self.make_hash(&k);
match self.search_hashed(&hash, &k) {
None => self.insert_hashed(hash, k, v),
None => {
let v = not_found(&k, a);
self.insert_hashed(hash, k, v)
},
Some(idx) => {
let (_, v_ref) = self.table.read_mut(&idx);
f(&k, v_ref);
found(&k, v_ref, a);
v_ref
}
}
Expand Down