How can I make it better

Hi all, this is my first post.
I'm asking for your review of my program for exercise 3 from chapter 8 in "The Book".

While writing solutions to those exercises from this chapter I constantly found myself thinking in JavaScript ways because this is my primary language at work for the past two years now. Would like to get rid of it to code in Rust way. I'm open to learning this brilliant language from this brilliant community. This is the code, thank you:

pub mod company {
    use std::{io, collections::HashMap};

    pub fn expand_company() {

        let mut departments = HashMap::new();

        departments.insert(String::from("Sales"), vec![String::from("Joe")]);
        departments.insert(String::from("Engineering"), vec![String::from("Lisa")]);

        let mut department = String::new();
        let mut name = String::new();
        
        println!("Enter a department's name:");

        read_into_location(&mut department);

        println!("Enter a person's name:");

        read_into_location(&mut name);

        department = title_case_words(&department);
        name = title_case_words(&name);

        println!("Adding {} to {} team.", name, department);

        match departments.get(&department) {
            None => {
                departments.insert(String::from(department), vec![name]);
            },
            Some(dep) => {
                let mut v = dep.to_vec();
                v.push(name);
                departments.insert(department, v);
            }
        }

        println!("{:?}", departments);
    }

    fn title_case_words(s: &str) -> String {
        let mut word = s.trim().chars();

        match word.next() {
            None => String::new(),
            Some(letter) => letter.to_uppercase().collect::<String>() + word.as_str(),
        }
    }

    fn read_into_location(loc: &mut String) {
        io::stdin()
            .read_line(loc)
            .expect("Failed to read name.");
    }
}
1 Like

Instead of the match departments.get(&department) block, you can use the HashMap's entry API:

departments
    .entry(department)
    .or_insert_with(Vec::new)
    .push(name);

This prevents having to look things up multiple times.

edit: Everything else there looks pretty good to me.

4 Likes

I agree with @asymmetrikon that everything here looks pretty good. As a personal preference, Iā€™d be more explicit about the types here:

As someone trying to understand the code, the key and value type of this map is pretty important. Stating them explicitly helps readers understand what purpose it serves:

let mut departments: HashMap<String, Vec<String>> = HashMap::new();
5 Likes

I like to look for places to use immutable types. Here, for example, I wonder if the Hash keys could be &str instead of String? It looks like that'd DRY up your code too, where you're converting from &str to String to create the keys...

1 Like

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.