Correct answer at rustlings course, but not happy with it-CodePudding

This is complaining on highest of niveaus, but I solved a task from the rustlings course and I'm confident that this can't be the optimal solution - or even a good one.

The task: https://github.com/rust-lang/rustlings/blob/main/exercises/hashmaps/hashmaps3.rs

My solution (only the the relevant bit):

fn build_scores_table(results: String) -> HashMap<String, Team> {
    // The name of the team is the key and its associated struct is the value.
    let mut scores: HashMap<String, Team> = HashMap::new();

    for r in results.lines() {
        let v: Vec<&str> = r.split(',').collect();
        let team_1_name = v[0].to_string();
        let team_1_score: u8 = v[2].parse().unwrap();
        let team_2_name = v[1].to_string();
        let team_2_score: u8 = v[3].parse().unwrap();

        let team_1 = scores.entry(team_1_name.clone()).or_insert(Team {
            name: team_1_name.clone(),
            goals_scored: 0,
            goals_conceded: 0,
        });
        team_1.goals_scored  = team_1_score;
        team_1.goals_conceded  = team_2_score;

        let team_2 = scores.entry(team_2_name.clone()).or_insert(Team {
            name: team_2_name.clone(),
            goals_scored: 0,
            goals_conceded: 0,
        });
        team_2.goals_scored  = team_2_score;
        team_2.goals_conceded  = team_1_score;
    }
    scores
}

My problem is, that I'm cloning the strings (twice!) inside the .entry() method an also in the Team struct. I tried using it without, but it's not working (borrowing stuff) and using the & but it's not happy because it expects String - not &String.

CodePudding user response：

Not sure what's not working exactly? You can just move on the second use site, it works fine:

        let team_1 = scores.entry(team_1_name.clone()).or_insert(Team {
            name: team_1_name,
            goals_scored: 0,
            goals_conceded: 0,
        });
        team_1.goals_scored  = team_1_score;
        team_1.goals_conceded  = team_2_score;

If you want zero cloning in the success case (where there's already an entry for the team) the code is a little less sexy but it also compiles fine:

        if let Some(t) = scores.get_mut(&team_1_name) {
            t.goals_scored  = team_1_score;
            t.goals_conceded  = team_2_score;
        } else {
            scores.insert(team_1_name.clone(), Team {
                name: team_1_name,
                goals_scored: team_1_score,
                goals_conceded: team_2_score,
            });
        }

Technically we can even remove the initial to_string and do no allocation in the hit case (obviously that means two allocations in the no-hit case):

        let team_1_name = v[0];
        let team_1_score: u8 = v[2].parse().unwrap();
        // ...
        if let Some(t) = scores.get_mut(team_1_name) {
            t.goals_scored  = team_1_score;
            t.goals_conceded  = team_2_score;
        } else {
            scores.insert(team_1_name.to_string(), Team {
                name: team_1_name.to_string(),
                goals_scored: team_1_score,
                goals_conceded: team_2_score,
            });
        }

Alternatively, remove the name from the Team struct, it's not really valuable since you have the hashmap key. Though at this point you don't have a Team struct anymore so it should probably be renamed e.g. Score or Goals (and you can strip the prefix from the two members left).

CodePudding user response：

You don't need the second .clone() calls (in the Team), because you can allow the struct to take ownership of that one, as nothing else uses it afterwards. They set the hashmap up kinda weird for you, since the key and the value both contain the team name. If you just remove it from the struct definition, there isn't any more need for cloning (it is never used, anyways).

struct Team {
    goals_scored: u8,
    goals_conceded: u8,
}

fn build_scores_table(results: String) -> HashMap<String, Team> {
    // The name of the team is the key and its associated struct is the value.
    let mut scores: HashMap<String, Team> = HashMap::new();

    for r in results.lines() {
        let v: Vec<&str> = r.split(',').collect();
        let team_1_name = v[0].to_string();
        let team_1_score: u8 = v[2].parse().unwrap();
        let team_2_name = v[1].to_string();
        let team_2_score: u8 = v[3].parse().unwrap();

        let team_1 = scores.entry(team_1_name).or_insert(Team {
            goals_scored: 0,
            goals_conceded: 0,
        });
        team_1.goals_scored  = team_1_score;
        team_1.goals_conceded  = team_2_score;

        let team_2 = scores.entry(team_2_name).or_insert(Team {
            goals_scored: 0,
            goals_conceded: 0,
        });
        team_2.goals_scored  = team_2_score;
        team_2.goals_conceded  = team_1_score;
    }
    scores
}