Rust book chapter 8 add employee exercise

I am a beginner would like to hear some feedback on my code.
Thank you, guys!

// Using a hash map and vectors,
// create a text interface to allow a user to add employee names to a department in a company.
// For example, “Add Sally to Engineering” or “Add Amir to Sales.”
// Then let the user retrieve a list of all people in a department
// or all people in the company by department,
//  sorted alphabetically.

use std::collections::HashMap;
use std::io;

fn main() {
    let mut emp_list = HashMap::new();

    loop {
        println!("Select a number:");
        println!("[1] Add Employee");
        println!("[2] Retrieve a list");
        println!("[3] Quit");

        let mut response = String::new();
        io::stdin().read_line(&mut response).expect("error");
        let response = response.trim();

        match response {
            "1" => {
                println!("Input Add <EMPLOYEE NAME> to <DEPARTMENT>");

                let mut emp_rec = String::new();
                io::stdin().read_line(&mut emp_rec).expect("error");
                let emp_rec = emp_rec.trim();
                let emp_rec_count = emp_rec.split_ascii_whitespace().count();
                
                if emp_rec_count > 3 {
                    let emp_rec: Vec<&str> = emp_rec.split_ascii_whitespace().collect();
                    let emp_name = emp_rec.get(1).unwrap().to_string();
                    let dept = emp_rec.get(3).unwrap().to_string();
                    
                    emp_list.insert(emp_name, dept);
                } else {
                    println!("Format should be 'Add <EMPLOYEE NAME> to <DEPARTMENT>'");
                }
            }
            "2" => {
                println!("Select a number:");
                println!("[1] list of all people in a department");
                println!("[2] all people in the company by department, sorted alphabetically");
                println!("[3] Quit");

                let mut response = String::new();
                io::stdin().read_line(&mut response).expect("error");
                let response = response.trim();

                match response {
                    "1" => {
                        println!("Input department to retrieve a list of all people in a department:");
                        let mut dept = String::new();
                        io::stdin().read_line(&mut dept).expect("error");
                        let dept = dept.trim();
                        
                        for (k, v) in &emp_list {
                            if dept == v {
                                println!("{k} at {v}");
                            }
                        }
                    },
                    "2" => {
                        println!("All people in the company by department, sorted alphabetically");
                        
                        let mut emp_rec: Vec<String> = Vec::new();
                        for (k, v) in &emp_list {                            
                            emp_rec.push(format!("{k} at {v}"));
                        }
                        
                        emp_rec.sort();

                        println!("{:?}", emp_rec);
                    },
                    "3" => break,
                    _ => (),
                }
            }
            "3" => break,
            _ => (),
        }
    }
}

Please fix your formatting:

2 Likes

You are not really making use of the kinds of things that a HashMap is good at, you only do either an insert or iterate over the whole dataset. You could try to change the representation of the data such that the operation to “retrieve a list of all peaople in a department” doesn’t have to go through all the data even in unrelated departments. One possible way would be e.g. to use the department as a key, and use a vector of employees in that department for as a value in the map.

Your input handling silently ignores parts of the input when more than 4 words are provided to the "Input Add <EMPLOYEE NAME> to <DEPARTMENT>" command. Either disallow more than 4 words, or try splitting up multiple words at the word "to" to allow employee-names and/or department names to contain spaces. It also doesn’t currently properly check for the usage of the words “Add” and “to”.

Inputting an existing employee in a different departments currently overwrites existing data. It would be nice if there was some kind of feedback when data already exists.

If you want to look beyond just HashMap and Vec, you could take a look at HashSet (which is basically a keys-only version of HashMap). Then instead of the approach of using HashMap<String, Vec<String>> to map departments to lists of employees, as I described above, you could use HashMap<String, HashSet<String>>. If you’re interested, also take a look at BTreeMap and BTreeSet in the standard library, which have similar API, but could be useful in this use-case because they already store their data in an alphabetically sorted order, so you could eliminate the need to introduce extra temporary Vecs and .sort() calls if you use those data structures instead of the hash-based ones.


Otherwise, in detail, the code looks decent. Ignoring any of the suggested changes to the main logic / data structure, on a smaller scale, optionally, you could use certain iterator operations instead of for loops, if you want. But not that that’s not really necessary, but mostly just a nice-to-know thing. E.g. the for ... { if ... { } } could be done using Iterator::filter method instead, and the for ... { emp_ref.push(...) } could be done using Iterator::map and Iterator::collect.

The latter thing is probably the only actually beneficial change, because that way the compiler can use the known size of the map to create a large enough Vec in the first place without re-allocations. Alternatively, initialize emp_rec as Vec::with_capacity(emp_list.len()) instead of Vec::new() to get the same kind of benefit. Using map and collect instead would look like

let mut emp_rec: Vec<String> = emp_list
    .iter()
    .map(|(k, v)| format!("{k} at {v}"))
    .collect();

emp_rec.sort();

Edit: Actually, I’m noticing you use debug-printing to show the list of employees. Debug-printing (i.e. via {:?}) is usually meant to only be used for debugging-purposes; you can try to create your own better-looking way of representing/listing the employees and departments with e.g. a loop over the Vec and multiple calls to println or print.

2 Likes

Thank you Alice!

In case I have to update the code can I just edit my post?

Thanks Steffahn for the great review! I will update my code and post an update here.

FYI, with regards to formatting, in this case I had already fixed your post for you :wink:

For new versions of the code with significant changes, a new reply might be more reasonable than editing the original, because future readers can make better sense if the discussion remains fairly linearly and earlier answers aren't reviewing code that no longer looks the way it looked like when it was reviewed. Of course ultimately you can do it however you prefer.

3 Likes

Thanks again!

Much better if I will post a new reply.

First HAPPY BIRTHDAY STEFFAHN! :partying_face:

Sorry for the very late reply had some challenging roadblocks in updating my code but fun for a newbie!

I used HashMap<String, Vec> here but as you have suggested, I am currently looking into HashSet, BTreeMap & BTreeSet!

I will update the code here again using HashSet, BTreeMap & BTreeSet.

Hope to get feedback on my updated code soon!

Thank you!

// Using a hash map and vectors,
// create a text interface to allow a user to add employee names to a department in a company.
// For example, “Add Sally to Engineering” or “Add Amir to Sales.”
// Then let the user retrieve a list of all people in a department
// or all people in the company by department,
//  sorted alphabetically.

use std::collections::HashMap;
use std::io;

fn main() {
    let mut emp_list: HashMap<String, Vec<String>> = HashMap::new();

    loop {
        println!("Select a number:");
        println!("[1] Add Employee");
        println!("[2] Retrieve a list");
        println!("[3] Quit");

        let mut response = String::new();
        io::stdin().read_line(&mut response).expect("error");
        let response = response.trim();

        match response {
            "1" => {
                println!("Input Add <EMPLOYEE NAME> to <DEPARTMENT>");

                let mut emp_rec = String::new();
                io::stdin().read_line(&mut emp_rec).expect("error");
                let emp_rec = emp_rec.trim();
                let emp_rec_count = emp_rec.split_ascii_whitespace().count();
                let word_count: usize = 4;

                if emp_rec_count < word_count {
                    println!("Format should be 'Add <EMPLOYEE NAME> to <DEPARTMENT>'");
                    continue;
                }

                let add_word = emp_rec
                    .split_ascii_whitespace()
                    .next()
                    .unwrap()
                    .to_ascii_lowercase();

                if add_word != "add" {
                    println!("'add' word missing as 1st word!");
                    println!("Format should be 'Add <EMPLOYEE NAME> to <DEPARTMENT>'");
                    continue;
                }

                let index_of_to_word = emp_rec
                    .split_ascii_whitespace()
                    .rev()
                    .position(|w| w.to_ascii_lowercase() == "to")
                    .unwrap_or_default();

                if index_of_to_word == 0 {
                    println!("'to' word missing between <EMPLOYEE NAME> and <DEPARTMENT>!");
                    println!("Format should be 'Add <EMPLOYEE NAME> to <DEPARTMENT>'");
                    continue;
                }

                // get employee name
                let index_of_to_word = emp_rec
                    .split_ascii_whitespace()
                    .position(|p| p.to_ascii_lowercase() == "to")
                    .unwrap();
                let emp_name: Vec<&str> = emp_rec
                    .split_ascii_whitespace()
                    .take(index_of_to_word)
                    .filter(|p| p.to_ascii_lowercase() != "add")
                    .collect();
                let emp_name = emp_name.join(" ");

                if emp_name.len() == 0 {
                    println!("Missing <EMPLOYEE NAME> between 'add' and 'to'");
                    println!("Format should be 'Add <EMPLOYEE NAME> to <DEPARTMENT>'");
                    continue;
                }

                // get department
                let index_of_to_word = emp_rec
                    .split_ascii_whitespace()
                    .position(|p| p.to_ascii_lowercase() == "to")
                    .unwrap();

                let dept: Vec<&str> = emp_rec
                    .split_ascii_whitespace()
                    .skip(index_of_to_word + 1)
                    .collect();
                let dept = dept.join(" ");

                // check if employee exist in department
                let employees_per_dept = emp_list.get(&dept);
                match employees_per_dept {
                    Some(_) => match employees_per_dept.filter(|p| p.contains(&emp_name)) {
                        Some(_) => {
                            println!("Employee already exist at {}!", dept);
                        }
                        None => {
                            emp_list
                                .entry(dept.to_ascii_lowercase())
                                .or_insert(Vec::new())
                                .push(emp_name);

                            println!("Successfully added employee!");
                        }
                    },
                    None => {
                        emp_list
                            .entry(dept.to_ascii_lowercase())
                            .or_insert(Vec::new())
                            .push(emp_name);

                        println!("Successfully added employee!");
                    }
                };
            }
            "2" => {
                println!("Select a number:");
                println!("[1] list of all people in a department");
                println!("[2] all people in the company by department, sorted alphabetically");
                println!("[3] Quit");

                let mut response = String::new();
                io::stdin().read_line(&mut response).expect("error");
                let response = response.trim();

                match response {
                    "1" => {
                        println!(
                            "Input department to retrieve a list of all people in a department:"
                        );

                        let mut dept = String::new();
                        io::stdin().read_line(&mut dept).expect("error");
                        let dept = dept.trim().to_ascii_lowercase();

                        if let None = emp_list.get(&dept) {
                            println!("Department does not exist!");
                            continue;
                        }

                        println!("List of all people in {}:", dept.to_ascii_uppercase());
                        for values in emp_list.get(&dept).unwrap() {
                            println!("{}", values.to_ascii_uppercase());
                        }
                    }
                    "2" => {
                        println!("All people in the company by department, sorted alphabetically");

                        for (dept, emp_name) in emp_list.iter_mut() {
                            emp_name.sort();

                            println!(
                                "{} department employees:\n{}\n",
                                dept.to_ascii_uppercase(),
                                emp_name.join("\n").to_ascii_uppercase()
                            );
                        }
                    }
                    "3" => break,
                    _ => (),
                }
            }
            "3" => break,
            _ => (),
        }
    }
}

FYI, I might not have the time (and/or strong motivation) to look through this right now, maybe someone else will have a look. And thanks for the birthday wishes.

Welcome! Thank you for the reply.

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.