3
\$\begingroup\$

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.

I've been seriously enjoying learning some Rust and reading the Rust book recently, and want to get a little feedback on my answer to this challenge from chapter 8.

I've deliberately kept things fairly short and simple - I've answered the question very literally and all of the criteria are met as far as I can tell from my testing.

I hard-coded the departments into the hashmap and I didn't try very hard to handle edge cases or improper input. Users also have to type in very clunky commands with exact wording.

The interface is not super user-friendly as after every command the loop restarts and the welcome message is shown each time.

Feel free to provide feedback on any of the above but know that I am at least aware of these issues.

I'm mostly concerned about idiomatic Rust and code structure.

Is there a better way for me to handle the various conditions than with if / else if / else?

I didn't want to nest too many loops in the main function for readability and my own sanity's sake. If I wanted to make the UX a little more seamless would it make sense to create a function for each command that might be called repeatedly i.e Add employees, View department, with an internal loop?

Anything else that stands out that I'm doing completely wrong?

Anything that stands out that I'm doing right?

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

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

    company.insert("engineering".to_string(), Vec::new());
    company.insert("operations".to_string(), Vec::new());
    company.insert("sales".to_string(), Vec::new());

    loop {
        println!("Welcome! What would you like to do?\n 
                  To add employee to a department, type 'Add <employee name> to <department name> (engineering, operations, or sales),\n 
                  To view employees in a department, type 'View department <department name>',\n
                  To view all employees by department, type 'View all',
                  To exit, type 'exit'.
        ");

        let mut entry = String::new();
        io::stdin()
            .read_line(&mut entry)
            .expect("Failed to read line");

        let entry: &str = entry.trim();
        let mut split = entry.split_whitespace();

        if entry.contains("Add") {
            let emp = split.nth(1).unwrap().to_string();
            let dep = split.nth(1).unwrap().to_lowercase();
            println!("{emp} added to {dep}");

            let vec = company.get_mut(&dep).unwrap();
            vec.push(emp);
            vec.sort_by_key(|name| name.to_lowercase());
            println!("{:?}", company)
        } else if entry.contains("View department") {
            let dep = split.nth(2).unwrap().to_lowercase();
            let view = &company[&dep];

            println!("Employees in {dep}:\n");
            for v in view {
                println!("{v}\n");
            }
        } else if entry.contains("View all") {
            for k in company.keys() {
                let view = &company[k];
                println!("Department: {k}");
                for v in view {
                    println!("{v}\n");
                }
            }
        } else if entry == "exit" {
            break;
        } else {
            println!("Please enter a valid option");
        }
    }
}
\$\endgroup\$
2
  • 2
    \$\begingroup\$ The entry.contains("Add") expression is a bit too permissive -- it matches View department Addiction Treatment Services. Better to ask if entry starts with "Add ". Also, in dep = split.nth(1)..., it would seem like we want nth(2), but I'm sure you've tested it so I guess not. Oh, and listen to everything @corvus_192 says in that excellent review. \$\endgroup\$
    – J_H
    Feb 10 at 16:58
  • \$\begingroup\$ @J_H Thanks for your comment! Agreed the entry.contains("Add") is way too broad, I appreciate the failure mode example you provided. For the dep = split.nth(1) , it has to be that way instead of a higher nth() because after emp = split.nth(1) the first two indices of split get consumed! It's an interesting behavior that definitely tripped me up for a bit while I was working it out. \$\endgroup\$ Feb 10 at 20:42

1 Answer 1

2
\$\begingroup\$

It's already very good, but there are a few thing I would change. First, I introduced different methods to make main a little shorter.

fn view_department(company: &HashMap<String, Vec<String>>, department: &str) {
    println!("Employees in {department}:\n");
    for e in &company[department] {
        println!("{e}\n");
    }
}

fn view_all(company: &HashMap<String, Vec<String>>) {
    for (department, employees) in company {
        println!("Department: {department}");
        for e in employees {
            println!("{e}\n");
        }
    }
}

fn add_employee(company: &mut HashMap<String, Vec<String>>, employee: &str, department: &str) {
    println!("{employee} added to {department}");

    let vec = company.get_mut(department).unwrap();
    vec.push(employee.to_owned());
    vec.sort_by_key(|name| name.to_lowercase());
    println!("{company:?}")
}

Notice that I iterate directly over the key-value-pairs of the hashmap, which is slightly more efficient than hashing twice.

You can simply use a multiline string to get rid of the newline escapes:

        println!(
"Welcome! What would you like to do?
To add employee to a department, type 'Add <employee name> to <department name> (engineering, operations, or sales),
To view employees in a department, type 'View department <department name>',
To view all employees by department, type 'View all',
To exit, type 'exit'."
        );

And most importantly, as the problem is about matching of different string patterns, I used pattern matching for this:

match entry.split_whitespace().collect::<Vec<&str>>().as_slice() {
    ["Add", emp, "to", dep] => add_employee(&mut company, emp, dep),
    ["View", "department", dep] => view_department(&company, dep),
    ["View", "all"] => view_all(&company),
    ["exit"] => break,
    _ => println!("Please enter a valid option")
}

I think this is more readable than the version with a lot of nth and magic indices.

Side note: I originally wrote entry.trim().split_whitespace(), but clippy informed me that the trim is unnecessary, so use clippy often and listen to its advice.

Here is the full main for reference:

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

    company.insert("engineering".to_string(), Vec::new());
    company.insert("operations".to_string(), Vec::new());
    company.insert("sales".to_string(), Vec::new());

    loop {
        println!(
"Welcome! What would you like to do?
To add employee to a department, type 'Add <employee name> to <department name> (engineering, operations, or sales),
To view employees in a department, type 'View department <department name>',
To view all employees by department, type 'View all',
To exit, type 'exit'."
        );

        let mut entry = String::new();
        io::stdin()
            .read_line(&mut entry)
            .expect("Failed to read line");

        match entry.split_whitespace().collect::<Vec<&str>>().as_slice() {
            ["Add", emp, "to", dep] => add_employee(&mut company, emp, dep),
            ["View", "department", dep] => view_department(&company, dep),
            ["View", "all"] => view_all(&company),
            ["exit"] => break,
            _ => println!("Please enter a valid option")
        }
    }
}
```
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Wow @corvus_192! Thank you so much for the review and the many very helpful suggestions. This is exactly what I was looking for. I especially like what you've done with the match statement. Rest assured I'll be dissecting every piece so I can hopefully absorb some of this into my arsenal. \$\endgroup\$ Feb 10 at 20:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.