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.")
}
1 Answer 1
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 return
s 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
struct
s 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.
-
\$\begingroup\$ This is detailed and helpful, thanks a lot. Using the indoc crate especially is one tip I hadn't even considered. \$\endgroup\$Jahwi– Jahwi2020年08月08日 16:13:15 +00:00Commented Aug 8, 2020 at 16:13
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\$