Why does refactoring by extracting a method trigger a borrow checker error?

The compiler is right to prevent you from borrowing the HashMap twice. Suppose in handle_user_data() you also tried to borrow self.users. You would break the rules of borrowing in Rust because you already have a mutable borrow on it and you can only have one.

Since you can't borrow self twice for your handle_user_data(), I will propose a solution. I don't know if it's the best, but it works without unsafe and without overhead (I think).

The idea is to use an intermediate struct that will borrow the other fields of self:

impl UserHandler {
    fn handle_data(&mut self, user_id: i32, data: &str) {
        if let Some(user) = self.users.get_mut(&user_id) {
            Middle::new(&mut self.counter).handle_user_data(user, data);
        }
    }
}

struct Middle<'a> {
    counter: &'a mut i32,
}

impl<'a> Middle<'a> {
    fn new(counter: &'a mut i32) -> Self {
        Self {
            counter
        }
    }

    fn handle_user_data(&mut self, user: &mut User, data: &str) {
        user.send("Message received!");
        *self.counter += 1;
    }
}

This way, the compiler knows we can't borrow users twice.

If you have only one or two things to borrow, a quick solution is to have an associated function that takes them as parameters:

impl UserHandler {
    fn handle_user_data(user: &mut User, data: &str, counter: &mut i32) {
        // ...
    }
}

We can improve this design:

struct UserHandler {
    users: HashMap<i32, User>, // Maps user id to User objects.
    middle: Middle,              // Represents internal state
}

impl UserHandler {
    fn handle_data(&mut self, user_id: i32, data: &str) {
        if let Some(user) = self.users.get_mut(&user_id) {
            self.middle.handle_user_data(user, data);
        }
    }
}

struct Middle {
    counter: i32,
}

impl Middle {
    fn new(counter: i32) -> Self {
        Self {
            counter
        }
    }

    fn handle_user_data(&mut self, user: &mut User, data: &str) {
        user.send("Message received!");
        self.counter += 1;
    }
}

Now we are sure we don't have overhead and the syntax is much cleaner.

Further information can be found in Niko Matsakis' blog post After NLL: Interprocedural conflicts. Mapping this answer to the blog post:

  • solution #1 -> "View structs as a general, but extreme solution" section
  • solution #2 -> "Free variables as a general, but extreme solution" section (here expressed as an associated function)
  • solution #3 -> "Factoring as a possible fix" section

When calling self.handle_user_data, you're taking the whole self mutably while still having a mutable borrowed user object, which Borrow Checker doesn't like. You can't have two mutable borrows as the same time.

One method to circumvent this is not take whole self mutably but take the counter mutably:

impl UserHandler {
    fn handle_data(&mut self, user_id: i32, data: &str) {
        if let Some(user) = self.users.get_mut(&user_id) {
            handle_user_data(user, data, &mut self.counter);
        }
    }
}

fn handle_user_data(user: &mut User, data: &str, counter: &mut i32) {
    user.send(data);
    *counter += 1;
}