Update Hashmap in a Struct via Method

#1

Using this code

use std::collections::HashMap; 
 
struct X {
    things: HashMap<String, Vec<String>>,
}   

impl X {
    fn add_to_things(&mut self, key: &str, value: &str) {
        let elements = self.things.entry(key.to_string()).or_default();
        elements.push(value.to_string());
        self.things.insert(key.to_string(), elements.to_vec());
    }
}   

fn main() {}

I get the follwing error:

error[E0499]: cannot borrow `self.things` as mutable more than once at a time
  --> src/main.rs:11:9
   |
9  |         let elements = self.things.entry(key.to_string()).or_default();
   |                        ----------- first mutable borrow occurs here
10 |         elements.push(value.to_string());
11 |         self.things.insert(key.to_string(), elements.to_vec());
   |         ^^^^^^^^^^^                         -------- first borrow later used here
   |         |
   |         second mutable borrow occurs here

error: aborting due to previous error

I think I understand where this error comes from and what it means but apparently I don’t understand borrowing well enough to come up with a working solution without returning a new struct. Is the whole concept wrong or is there a better solution?
Also feel free to point out any stylistic mistakes.
Thanks.

0 Likes

#2

Just build the vector before .insert() call.

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=1e390f9b2ca919e87e90e3d977f9a7cd

Anyway, why you need that insert call? In your example it just replace already-modified vector with clone of itself, semantically nothing except bunch of memory allocations and memcpy.

0 Likes

#3

I think you could do this? But I’m new to rust as well so I’m not sure.


struct X {
    things: HashMap<String, Vec<String>>,
}

impl X {
    fn add_to_things(&mut self, key: &str, value: &str) {
        let elements = self.things.entry(key.to_string()).or_insert(vec![]);
        elements.push(value.to_string());
    }
}
1 Like

#4

You are right! I absolutely did not realize that. Thank you for the hint.

0 Likes

#5

This raises a question on its own, regarding the cloning of the key: Owned key argument or double hash_map lookup?

Which one is better?

  • Given the following type:

    type Things = ::std::collections::HashMap<String, Vec<String>>;
    
  • Double lookup:

    fn add_to_things (
        things: &'_ mut Things,
        key: &'_ str,
        value: String,
    )
    {
        if let Some(elements) = things.get_mut(key) {
            elements.push(value);
        } else {
            things.insert(key.to_owned(), vec![value]);
        }
    }
    
  • Requiring an owned key:

    fn add_to_things (
        things: &'_ mut Things,
        key: String,
        value: String,
    )
    {
        things
            .entry(key)
            .or_default()
            .push(value)
        ;
    }
    

I think that requiring an owned key is such a performance loss that the redundant lookup is negligible compared to it, but I might be wrong.

Anyways, couldn’t there be in the API something along these lines?

fn entry_cow<Q> (
  self: &'_ mut HashMap<K, V, S>,
  key_view: &'_ Q,
) -> Entry<'_, K, V>
where
    K : Borrow<Q>,
    Q : ToOwned<Owned = K> + Hash + Eq + ?Sized,

(or a impl FnOnce(&Q) -> K argument instead of the Q : ToOwned<Owned = K> bound (since we would always be able to feed Q::to_owned in that case))

0 Likes

#6

This is the type of thing the raw_entry API is supposed to address - currently unstable. This discussion precipitated it.

2 Likes

#7

@vitalyd I hadn’t looked at the raw entry API since it looked too fearsome for something as basic as to-owned-on-insert, but the discussion you linked to is very informative, thanks :slight_smile:

0 Likes