7
\$\begingroup\$

I recently finished chapter 8 of Rust's book, and below is my solution to the third exercise:

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'd appreciate pointers on how the code can be improved. Thanks in advance.

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

//An attempt at Rust book's chapter 8's third exercise:
//https://doc.rust-lang.org/book/ch08-03-hash-maps.html

fn main() {
    println!("The Office - Text Interface.");
    println!();
    println!("Enter a query, type HELP for a list of keyword and their functions, or type EXIT to exit.");
    println!();

    //build hashmap{department: vec[names]} database, insert default values
    let mut company = HashMap::new();
    
    let depts = vec!["SALES", "ENGINEERING", "HR", "SANITATION"];

    let sales = vec!["Sally", "Jordan", "Charlie", "Abigail"];
    let engineering = vec!["Suzy", "Jay", "Chi", "Amy"];
    let hr = vec!["Son", "Jack", "Chia", "Anna"];
    let sanitation = vec!["August", "Entangle", "Will", "Jada"];
    let tup = [sales, engineering, hr, sanitation];

    let mut g: Vec<_> = Vec::new();
    company = depts.into_iter()
    .map(|x| x.to_string())
    .zip(tup.iter().map(|y| {g = y.iter().map(|q| q.to_string()).collect(); g.clone()}))
    .collect();

    let keywords = ["ADD", "LIST", "UPDATE", "REMOVE", "HELP", "EXIT"];

    // loop the input part of the text interface.
    //validate first keyword, send queries to functions.
    loop {
        let mut query = String::new();
        println!("::");

        //check for empty input
        io::stdin().read_line(&mut query).expect("Enter a valid input");
        query = query.trim().to_string();
        // println!("{}", query);
        if query.is_empty() {
            println!("Invalid input. Type HELP for a keyword reference.");
            continue;
        }

        //check for valid first keyword
        let keyword = query.split_whitespace().next().unwrap().to_uppercase();
        if !keywords.contains(&&keyword[..]) {
            println!("Invalid Keyword. Type HELP for a keyword reference.");
            continue;
        }

        //keyword validated. Call the function.
        let mut query = query.split_whitespace().collect::<Vec<_>>();
        match &&keyword[..] {
            &"EXIT" => return,
            &"HELP" => help(),
            &"ADD" => add(&mut query, &mut company),
            &"LIST" => list(&mut query, &mut company),
            &"UPDATE" => update(&mut query, &mut company),
            &"REMOVE" => remove(&mut query, &mut company),
            _ => (),
        }
        // println!("{:?}", company); //debug purposes: print the entire hashmap on each loop to monitor changes.
        continue;
    }
}

fn add(q: &mut Vec<&str>, company: &mut HashMap<String, Vec<String>>) {

    //validate add syntax
    let length = q.len();
    if length < 3 || length > 4 {
        println!("Invalid ADD syntax. Type HELP for a keyword reference.");
        return;
    }

    //add a new department
    if length == 3 {
        match (q[0], q[1], q[2]) {
            ("ADD", "-D", d) => {

                //check if dept exists
                let dept = d.to_uppercase();
                if company.contains_key(&dept) {
                    println!("Department {} already exists.", d);
                    return;
                }

                //add dept
                company.entry(dept).or_insert(Vec::new());
                println!("Created department {}.", d);
                return;

            }

            _ => {
                println!("Invalid syntax.");
                return;
            }
        }
    }

    //add a person to a department
    if length == 4 {
        match (q[0], q[1], q[2], q[3]) {
            ("ADD", name, "TO", d) => {

                //check if dept exists
                let dept = d.to_uppercase();
                if !company.contains_key(&dept) {
                    println!("Department {} does not exist.", d);
                    return;
                }

                //check if name already exists in dept
                if company[&dept].contains(&name.to_owned()) {
                    println!("The name {} already exists in {}.", name, dept);
                    return;
                }
                //add name to vector
                (*company.get_mut(&dept).unwrap()).push(name.to_owned());
                println!("Added {} to {}.", name, d);
            }
            _ => {
                println!("Invalid Syntax");
                return;
            }
        }
    }
}

fn list(q: &mut Vec<&str>, company: &mut HashMap<String, Vec<String>>) {

    //sanitize input
    let length = q.len();
    if length != 2 && length !=4 {
        println!("Invalid number of arguments.");
        return;
    }

    if length == 2 {
        match (q[0], q[1]) {

            //list all depts
            ("LIST", "-D") => {
                let mut depts: Vec<_> = company.keys().collect();
                depts.sort();
                for d in depts {
                    println!("{}", d);
                }
                return;
            }

            //list everyone in all depts, sorted alphabetically
            ("LIST", "-E") => {
                for (dept, mut names) in company.clone() {
                    println!("---{}---", dept);
                    names.sort();
                    for name in names {
                        println!("{}", name);
                    }
                }
            }
            _ => {
                println!("Invalid Syntax.");
                return;
            }
        }
    }

    if length == 4 {
        match (q[0], q[1], q[2], q[3]) {
            ("LIST", "-E", "IN", d) => {

                //check if dept exists
                let dept = d.to_uppercase();
                if !company.contains_key(&dept) {
                    println!("Department {} does not exist.", d);
                    return;
                }

                //list all in department
                println!("---{}---", dept);
                (*company.get_mut(&dept).unwrap()).sort();
                for name in &company[&dept] {
                    println!("{}", name);
                }
            }
            _ => {
                println!("Invalid Syntax.");
                return;
            }
        }
    }
}

fn update(q: &mut Vec<&str>, company: &mut HashMap<String, Vec<String>>) {
    let length = q.len();

    if length != 5 && length != 6 {
        println!("Invalid UPDATE syntax.");
        return;
    }

    if length == 5 {
        match (q[0], q[1], q[2], q[3], q[4]) {

            //update a department
            ("UPDATE", "-D", old_d, "TO", new_d) => {

                //check if dept exists
                let old_dept = old_d.to_uppercase();
                let new_dept = new_d.to_uppercase();
                if !company.contains_key(&old_dept) {
                    println!("Department {} does not exist.", old_d);
                    return;
                }
                if company.contains_key(&new_dept) {
                    println!("Department {} already exists.", new_d);
                    return;
                }

                //rename dept. Technique is to build a new vector with that same name since you
                //cannot change the key of a hash map.
                let temp_dept = company.get(&old_dept).unwrap().clone();
                company.insert(new_dept.to_uppercase(), temp_dept);
                company.remove(&old_dept);
                println!("Changed Department {} to {}.", old_d, new_d);
                return;

            }
            _ => {
                println!("Invalid syntax.");
                return;
            }
        }
    }

    //change a name in a department
    match (q[0], q[1], q[2], q[3], q[4], q[5]) {
        ("UPDATE", old_name, "FROM", d, "TO", new_name) => {

            //check if dept exists
            let dept = d.to_uppercase();
            if !company.contains_key(&dept) {
                println!("Department {} does not exist.", d);
                return;
            }

            //check if old name and new name exist
            if !company[&dept].contains(&old_name.to_owned()) {
                println!("The name {} does not exist in {}.", old_name, dept);
                return;
            }
            if company[&dept].contains(&new_name.to_owned()) {
                println!("The name {} already exists in {}.", new_name, dept);
                return;
            }

            //update the name.
            for (i, name) in company[&dept].clone().iter().enumerate() {
                if name == old_name {
                    (*company.get_mut(&dept).unwrap())[i] = new_name.to_owned();
                    println!("Changed {} in {} to {}.", old_name, dept, new_name);
                    return;
                }
            }
        }
        _ => {
            println!("Invalid Syntax.");
            return;
        }
    }
}

fn remove(q: &mut Vec<&str>, company: &mut HashMap<String, Vec<String>>) {
    let length = q.len();

    if length !=3 && length !=4 {
        println!("Invalid REMOVE syntax.");
        return;
    }

    if length == 3 {
        match (q[0], q[1], q[2]) {
            ("REMOVE", "-D", d) => {

                //check if dept exists
                let dept = d.to_uppercase();
                if !company.contains_key(&dept) {
                    println!("Department {} does not exist.", d);
                    return;
                }

                //remove the department.
                company.remove(&dept);
                println!("Removed department {}.", d);
                return;

            }
            _ => {
                println!("Invalid Syntax.");
                return;
            }
        }
    }

    //remove a person
    match (q[0], q[1], q[2], q[3]) {
        ("REMOVE", name, "FROM", d) => {

            //check if dept exists
            let dept = d.to_uppercase();
            if !company.contains_key(&dept) {
                println!("Department {} does not exist.", d);
                return;
            }

            //check if name exists
            if !company[&dept].contains(&name.to_owned()) {
                println!("The name {} does not exist in {}.", name, dept);
                return;
            }

            //remove the name
            for (i, _name) in company[&dept].clone().iter().enumerate() {
                if _name == name {
                    (*company.get_mut(&dept).unwrap()).remove(i);
                    println!("Removed {} from {}.", name, dept);
                    return;
                }
            }

        }
        _ => {
            println!("Invalid Syntax.");
            return;
        }
    }
}

fn help() {
    println!("The Office - KEYWORD HELP");
    println!();
    println!("Note: All keywords are case-sensitive.");
    println!("Keywords: \nLIST - Lists items in the database");
    println!("Usage:    LIST -E - Lists all employees");
    println!("          LIST -E IN [DEPARTMENT] - Lists all employees in specified department.");
    println!("          LIST -D - Lists all departmnets in the company");
    println!();
    println!("ADD -     Adds items to the database.");
    println!("Usage:    ADD [name] TO [department] - Adds the name to the specified department.");
    println!("          ADD -D [department] - Adds the department to the roster.");
    println!();
    println!("REMOVE -  Removes items from the database.");
    println!("          REMOVE -D [department] - Removes the particular department from the database.");
    println!("          REMOVE [name] FROM [department] - Removes the person from the specified department.");
    println!();
    println!("UPDATE -  Changes records in the database.");
    println!("Usage:    UPDATE -D [old name] TO [new name] - Changes a department's name.");
    println!("          UPDATE [old name] FROM [department] TO [new name] - Changes a person's name.");
    println!();
    println!("HELP -    Prints this help screen.");
    println!();
    println!("EXIT -    Exits the program.")
}
\$\endgroup\$
1
  • 3
    \$\begingroup\$ To improve readability, run cargo fmt in your project root. This will run rustfmt which will automatically format your code according to the official code style. If you want, you can configure rustfmt to behave differently \$\endgroup\$
    – MindSwipe
    Aug 7, 2020 at 11:03

1 Answer 1

4
\$\begingroup\$

Welcome to Code Review.

Formatting

The first thing I did to your code was to apply rustfmt by typing cargo fmt. rustfmt formats your code to fit with Rust's standard formatting guidelines. Here's some notable changes.

-    company = depts.into_iter()
-    .map(|x| x.to_string())
-    .zip(tup.iter().map(|y| {g = y.iter().map(|q| q.to_string()).collect(); g.clone()}))
-    .collect();
+    company = depts
+        .into_iter()
+        .map(|x| x.to_string())
+        .zip(tup.iter().map(|y| {
+            g = y.iter().map(|q| q.to_string()).collect();
+            g.clone()
+        }))
+        .collect();

Method invocation chains are indented. Complex closures are formatted over several lines.

-    if length !=3 && length !=4 {
+    if length != 3 && length != 4 {

Most binary operators are surrounded by spaces.

Clippy

After that, cargo clippy pointed out some issues with your code.

warning: unneeded `return` statement
   --> src\main.rs:270:13
    |
270 |             return;
    |             ^^^^^^^ help: remove `return`
    |
    = note: `#[warn(clippy::needless_return)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return

warning: unneeded `return` statement
   --> src\main.rs:332:13
    |
332 |             return;
    |             ^^^^^^^ help: remove `return`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return

In Rust, functions automatically return when control flow reaches the end of the function body, so the explicit returns are unnecessary.

warning: value assigned to `company` is never read
  --> src\main.rs:16:9
   |
16 |     let mut company = HashMap::new();
   |         ^^^^^^^^^^^
   |
   = note: `#[warn(unused_assignments)]` on by default
   = help: maybe it is overwritten before being read?

You assigned an initial value to company, but overwrote it afterwards. It is recommended to postpone the declaration of company to the place you calculate it.

warning: you don't need to add `&` to both the expression and the patterns
  --> src\main.rs:64:9
   |
64 | /         match &&keyword[..] {
65 | |             &"EXIT" => return,
66 | |             &"HELP" => help(),
67 | |             &"ADD" => add(&mut query, &mut company),
...  |
71 | |             _ => (),
72 | |         }
   | |_________^
   |
   = note: `#[warn(clippy::match_ref_pats)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#match_ref_pats
help: try
   |
64 |         match &keyword[..] {
65 |             "EXIT" => return,
66 |             "HELP" => help(),
67 |             "ADD" => add(&mut query, &mut company),
68 |             "LIST" => list(&mut query, &mut company),
69 |             "UPDATE" => update(&mut query, &mut company),
 ...

Self explanatory.

warning: use of `or_insert` followed by a function call
  --> src\main.rs:98:37
   |
98 |                 company.entry(dept).or_insert(Vec::new());
   |                                     ^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(Vec::new)`
   |
   = note: `#[warn(clippy::or_fun_call)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#or_fun_call

.or_insert(Vec::new()) always constructs the vector; if the entry already exists, the newly constructed empty vector is discarded. A better alternative is .or_default(Vec::new). (It probably doesn't make a big difference in this case though.)

Structuring data

In Chapter 5 Using Structs to Structure Related Data, we learned to use structs and methods to organize our data. We can define some structs to clarify the meaning of our data:

#[derive(Clone, Debug)]
struct Department {
    employees: Vec<String>,
}

#[derive(Clone, Debug)]
struct Company {
    departments: HashMap<String, Department>,
}

And we can build the preset data in an associated function:

impl Company {
    fn preset() -> Self {
        let departments = &[
            ("SALES", &["Sally", "Jordan", "Charlie", "Abigail"]),
            ("ENGINEERING", &["Suzy", "Jay", "Chi", "Amy"]),
            ("HR", &["Son", "Jack", "Chia", "Anna"]),
            ("SANITATION", &["August", "Entangle", "Will", "Jada"]),
        ];

        Company {
            departments: departments
                .iter()
                .map(|&(name, department)| {
                    (
                        name.to_string(),
                        Department {
                            employees: department.iter().map(|&s| s.to_string()).collect(),
                        },
                    )
                })
                .collect(),
        }
    }
}

(Personally, I would prefer using serialization instead of hardcoding the preset data.)

Unnecessary allocation

In main, there is an unnecessary allocation:

query = query.trim().to_string();

You can simply make a reference into the original input:

let query = query.trim();

Note that shadowing is used here to maintain the variable that owns the original string.

Input parsing

You first check for empty input, and then use .next().unwrap(). Just use a match:

let query = query.trim();
let mut args = query.split_whitespace();

match args.next() {
    None => println!("Empty input. Type HELP for a keyword reference."),
    Some("ADD") => execute::add(args.collect(), &mut company),
    Some("EXIT") => return,
    Some("HELP") => help(),
    Some("LIST") => execute::list(args.collect(), &mut company),
    Some("REMOVE") => execute::remove(args.collect(), &mut company),
    Some("UPDATE") => execute::update(args.collect(), &mut company),
    Some(_) => println!("Invalid Keyword. Type HELP for a keyword reference."),
}

I put all the helper functions in a execute module. I also changed the parsing functions to take args by value. The keyword is excluded from the list of arguments.

add

Checking if a department exists can be done with the entry API:

let department = department.to_uppercase();
match departments.entry(&department) {
    Entry::Occupied(_) => println!("Department {} already exists.", d),
    Entry::Vacant(entry) => {
        entry.insert(Department::new());
        println!("Created department {}.", d);
    }
}

In fact, the whole function can be simplified with pattern matching:

pub fn add(args: &[&str], company: &mut Company) {
    let departments = &mut company.departments;

    match *args {
        ["-D", department] => {
            use std::collections::hash_map::Entry;

            let department = department.to_uppercase();

            match departments.entry(department) {
                Entry::Occupied(entry) => {
                    println!("Department {} already exists.", entry.key())
                }
                Entry::Vacant(entry) => {
                    println!("Created department {}.", entry.key());
                    entry.insert(Department::new());
                }
            }
        }
        [name, "TO", department] => {
            let department = department.to_uppercase();

            let employees = match departments.get_mut(&department) {
                None => {
                    println!("Department {} does not exist.", department);
                    return;
                }
                Some(department) => &mut department.employees,
            };

            if employees.iter().any(|employee| employee == name) {
                println!("The name {} already exists in {}.", name, department);
            } else {
                employees.push(name.to_string());
                println!("Added {} to {}.", name, department);
            }
        }
        _ => println!("Invalid syntax."),
    }
}

Other functions can be simplified in a similar fashion.

continue

Similar to the implicit return, you do not need to explicitly continue to the next iteration of the loop at the end of the loop body.

help

The indoc crate provides a nice way to write multiline string literals:

pub fn help() {
    println!(indoc! { r#"
        <fill in text here>
    "#})
}

The indentation common to every line will be stripped, and the rest of the indentation will be preserved.

\$\endgroup\$
1
  • \$\begingroup\$ This is detailed and helpful, thanks a lot. Using the indoc crate especially is one tip I hadn't even considered. \$\endgroup\$
    – Jahwi
    Aug 8, 2020 at 16:13

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.