Pattern to avoid repeating logic

help
#1 
use std::collections::hash_map::Entry::{Occupied, Vacant};
use std::collections::HashMap;

struct S {}

struct Foo {
    counter: u64,
    h: HashMap<u64, S>,
    g: HashMap<String, u64>,
}

impl Foo {
    fn add_h(&mut self) -> u64 {
        self.counter += 1;
        let s = S {};
        self.h.insert(self.counter, s);
        self.counter
    }

    fn add_g(&mut self, s: String) -> u64 {
        match self.g.entry(s) {
            Occupied(s) => *s.get(),
            Vacant(v) => {
                let h = self.add_h();
                *v.insert(h)
            }
        }
    }

    fn add_g2(&mut self, s: String) -> u64 {
        match self.g.entry(s) {
            Occupied(s) => *s.get(),
            Vacant(v) => {
                let h = {
                    self.counter += 1;
                    let s = S {};
                    self.h.insert(self.counter, s);
                    self.counter
                };
                *v.insert(h)
            }
        }
    }
}

The method add_g does not pass the borrow checker. It actually make sense, since I got self borrowed as mutable (by add_g) and then I try to borrow it again with add_h.

I can fix the problem with the method add_g2, that basically replace the call to add_h with the body of add_h itself (I made it explicit with a false scope). I don't like this approach since I am replicating logic that should not be replicated, IMHO.

Are there any pattern to avoid this problem?

#2

You should be able to do something along these lines (untested):

use std::collections::hash_map::Entry::{Occupied, Vacant};
use std::collections::HashMap;

struct S {}

struct Foo {
    counter: u64,
    h: HashMap<u64, S>,
    g: HashMap<String, u64>,
}

impl Foo {
    fn add_h_inner(counter: &mut u64, h: &mut HashMap<u64, S>) -> u64 {
        *counter += 1;
        let s = S {};
        h.insert(*counter, s);
        *counter
    }

    pub fn add_h(&mut self) -> u64 {
        Self::add_h_inner(&mut self.counter, &mut self.h)
    }

    pub fn add_g(&mut self, s: String) -> u64 {
        match self.g.entry(s) {
            Occupied(s) => *s.get(),
            Vacant(v) => {
                let h = Self::add_h_inner(&mut self.counter, &mut self.h);
                *v.insert(h)
            }
        }
    }
}
1 Like
#3

This solution works perfectly!

I don't love it, because the function is actually more complex than the one showed here, so it is necessary to pass several parameters, but still better than replicating the code.

#4

If those parameters are logically related, you could define a private struct containing just them, and have that struct as a field inside the main one.

#5

Unfortunately I don't believe that this approach scales well.

I am founding more complex functions now to implement, and all of them play very bad with the single borrow rule.

I basically have a central HashMap that continuously does, get or insert, next to it, few other HashMap with similar usage pattern.

I need more thinking... Or a better design

#6

Niko has a blog post about this issue. Three of the four ways around the problem that are discussed there (inlining, struct factoring, free variables) are already covered in this thread. The fourth way is "view structs", if you want to just jump to it.

2 Likes
#7

Thank you, it was a nice lecture.