9) downgrading mut refs to shared refs is safe: last example seems confusing

[BartMassey]

In 9) downgrading mut refs to shared refs is safe the last example starts with a block commented with // drop the returned mut Player refs since we can't use them together anyway. Commenting out this block still allows the code to compile (playground), and its purpose is unclear to me. I think it might be better to just remove it?

[pretzelhammer]

Thank you for the feedback! This is how I intended for that code example to be read:

use std::collections::HashMap;

type PlayerID = i32;

#[derive(Debug, Default)]
struct Player {
    score: i32,
}

fn start_game(player_a: PlayerID, player_b: PlayerID, server: &mut HashMap<PlayerID, Player>) {
    // players may or may not be in server at this point
    // so either fetch the existing players or insert
    // new player defaults
    server.entry(player_a).or_default();
    server.entry(player_b).or_default();

    // fetch the players again, getting them immutably this time, without any implicit re-borrows
    let player_a = server.get(&player_a);
    let player_b = server.get(&player_b);

    // do something with players
    dbg!(player_a, player_b); // compiles
}

fn main() {
    let mut server = HashMap::new();
    // generate player ids somehow and then run this function
    // don't assume players are already present in the hashmap
    start_game(1, 2, &mut server);
}

playground

The idea is that the start_game function adds the players to the server if they don't already exist in the server, because they may not exist yet. That's essentially what server.entry(player).or_default() does. If that line isn't included a new invariant is added to the function: all players MUST already be in the server or the function can't do it's job and must return a Result for the error case.

Does it make more sense now? Or is it still confusing? I might add some additional text to the article explaining the purpose of the lines if more people find it confusing.

[BartMassey]

The idea is that the start_game function adds the players to the server if they don't already exist in the server, because they may not exist yet.

Ah. This is what I missed.

I would perhaps phrase the comment something like

    // a player may not have a record on the server when this function is called.
    // if not, fetch the player mutably and insert a default record for them, then
    // drop the mutable reference

This makes it clear why or_default() is being called: I forgot that it has a side-effect, and was confused why we were looking up the players and throwing the result away. I might also modify the second comment to something like

    // fetch the players immutably this time so that both players' references can be
    // used together

Thanks much for a great document! I will be using it in my Rust class going forward.

[pretzelhammer]

I left a small comment in the code example in the article that hopefully makes it clear that or_default() performs a side-effect of inserting the players into the server.

thanks for your feedback!