rust-lang / rfcs

RFCs for changes to Rust
https://rust-lang.github.io/rfcs/
Apache License 2.0
5.79k stars 1.55k forks source link

Entry::insert #2104

Open eira-fransham opened 6 years ago

eira-fransham commented 6 years ago

I'm currently in a position of wanting an Entry::insert (or other bikesheddable name) for when you want to conditionally update a value - inserting a value if there is none and updating it if it exists but is "worse" by some metric. This would be nicely captured with code similar to the following:

let was_updated = match map.entry(key) {
    Entry::Occupied(ref entry) if !new_val.better_than(entry.get()) => false,
    otherwise => { otherwise.upsert(new_val); true },
};

Instead, you have to do:

let was_updated = match map.entry(key) {
    Entry::Occupied(ref entry) if !new_val.better_than(entry.get()) => false,
    Entry::Occupied(entry) => { entry.insert(new_val); true },
    Entry::Vacant(entry) => { entry.insert(new_val); true },
};

Which is redundant. The two insert methods have different signatures but I would imagine that Entry::insert would return an Result<&mut Value, Value> to pass ownership in the case that the entry is occupied (or maybe a CowMut if that was to exist).

Nokel81 commented 6 years ago

can this not be done with something like the following?

let was_updated = match map.entry(key) {
    Entry::Occupied(ref entry) if !new_val.better_than(entry.get()) => false,
    (entry) _ => { entry.insert(new_val); true }
};

If it cannot be done, then this could be a good solution

eira-fransham commented 6 years ago

The insert calls have different type signatures, so although I don't know what the (entry) _ syntax is it wouldn't work either way.

bluss commented 6 years ago

Sounds reasonable. This is the small kind of improvement that you can normally contribute as a PR and the libs team will discuss it from there. User can fix it locally in the mean time:

use std::collections::hash_map::Entry;

pub trait InsertExt {
    type Item;
    fn insert(self, item: Self::Item);
}

impl<'a, K, V> InsertExt for Entry<'a, K, V> {
    type Item = V;
    fn insert(self, item: Self::Item) {
        match self {
            Entry::Occupied(mut e) => { e.insert(item); }
            Entry::Vacant(e) => { e.insert(item); } 
        }
    }
}
Diggsey commented 6 years ago

https://crates.io/crates/mucow