Description
openedon Mar 4, 2021
HashMap::insert(key, val)
returns Some(old_val)
as the 'error' if the key was already in the map. In many cases it's assumed no duplicate keys are ever added, but still asserted by panicking otherwise.
We experimented with map.insert(key, val).unwrap_none()
(rust-lang/rust#62633), but decided that that's not the kind of method we'd like to have on Option
s.
Using an assert macro has some downsides:
assert!(map.insert(key, val).is_none()); // 1
assert_eq!(map.insert(key, val), None); // 2
assert_match!(map.insert(key, val), None); // 3
Not everyone feels comfortable with assertions having side effects.
1
doesn't show any context in the panic message. (Though unwrap_none
also didn't show the key or the new value. Only the old value.) 2
would only work if the value type implements PartialEq
. 3
uses a macro that doesn't exist (yet) in std
.
Updating the HashMap
interface might be a good solution:
insert
always succeeds because it replaces the old value if it exists. One could argue that insert()
is never the right method for panicking on duplicates, since already handles that case by replacing the value, only allowing you to panic after that already happened.
A try_insert
method could instead return an Result::Err
indicating the duplicate key error, which could also report the full context (key, old value, new value) in the panic message.
map.try_insert(key, val).unwrap();
Alternatively (or additionally), the Entry
interface could be updated to have a .unwrap_vacant()
method to assume the obtained entry is vacant, which can then be used to insert the value. This can report the key and the old value in the panic message.
map.entry(key).unwrap_vacant().insert(val);