Code that "modifies" a value in a HashMap, is there a better way?

help
1

I have some code that generates, then modifies values in a HashMap. I actually don't modify the values, but instead create a second HashMap and populate it. The code works, but I find it ugly (especially the need for flag). Is there a better way? Thanks for any help / ideas. Note that the code below has been excerpted. The second Vec in h is used later on in the code.

    let mut h: HashMap<Rational32, ((i32, i32), Vec<i32>, Vec<i32>)> = HashMap::new();
    for a in 1..a_cc+1 {
        for b in 1..b_cc+1 {
            let m = Rational32::new(b * a_cc, a * b_cc);
            if m > Rational32::new(1,1) && !h.contains_key(&m) {
                h.insert(m, ((a, b), Vec::new(), Vec::new()));
            }
        }
    }

    let mut h2: HashMap<Rational32, ((i32, i32), Vec<i32>)> = HashMap::new();
    for a in 1..a_bc+1 {
        for b in 1..b_bc+1 {
            let m = Rational32::new(b * a_bc, a * b_bc);
            let mut v: Vec<i32> = vec![a, b];
            let mut flag = false;
            if let Some(&((ah, bh), _, _)) = h.get(&m) {
                if let Some(&((_, _), ref vh2)) = h2.get(&m) {
                    v = vh2.clone();
                    v.push(a);
                    v.push(b);
                    flag = true;
                }
                if flag {
                    h2.remove(&m);
                }
                h2.insert(m, ((ah, bh), v));
            }
        }
    }
    h.clear();
2

Normally you'd use HashMap::get_mut or HashMap::entry to make changes to values in the map.

3

Thanks @jethrogb, I should also explain that h has almost a million entries while h2 has about 30000. The code I used avoids having to find delete all the extraneous entries. I'm OK with creating the extra HashMap, but don't like the code structure.

4

Oh ok, you're basically just looking to clean up the code inside if let Some(...) = h.get(&m) { ... }. That code looks like a good candidate for the use of HashMap::entry.

5

It seems like you can change

if let Some(&((_, _), ref vh2)) = h2.get(&m) {
...
}

to

if let Some(((_, _), vh2)) = h2.remove(&m) {
...
}

So just try the removal upfront. That would remove the need for flag.

1 Like
6

Thanks @vitalyd, that is more elegant. Following @jethrogb's advice this is what I have now for the second code chunk:

    let mut h2: HashMap<Rational32, ((i32, i32), Vec<i32>)> = HashMap::new();
    for a in 1..a_bc + 1 {
        for b in 1..b_bc + 1 {
            let m = Rational32::new(b * a_bc, a * b_bc);
            let mut v: Vec<i32> = vec![a, b];
            if let Some(&((ah, bh), _, _)) = h.get(&m) {
                h2.entry(m)
                    .and_modify(|((_, _), vh2)| {
                        vh2.push(a);
                        vh2.push(b);
                    })
                    .or_insert(((ah, bh), v));
            }
        }
    }
    h.clear();
7

I think you can also make the code a bit less noisy by relying on type inference.

8

Can you show me an example of what you mean, @vitalyd? Thanks!

9

How about something like this: Rust Playground

?

10

Thanks @anderejd. That is definitely tighter and avoids all the ugly _'s but is code with t2.1.push() and the like preferred? I tried your idea out and another benefit is that it satisfies clippy; which had been complaining about complex types.

11

I'm not sure, in this case it was simply my personal preference given a couple of minutes to mess around with the example. If clippy is happy it's probably at least okayish :slight_smile: