Your questions
HashMap<String, Vec<String>>
is fine. Since this object exists in main
at the highest level, it must store owned values instead of references to its content, so String
and Vec
must be used instead of &str
.
For a not yet checked error case see below, under "Missing indexing check".
Remove slice_vec_to_string_vec
First of all, its name is misleading as it silently discards the first element of the input vec. It can be replaced in command_parser
by changing this line:
let args: Vec<&str> = c.split(' ').collect();
to:
let mut args: Vec<String> = c.split(' ').map(String::from).collect();
This directly converts all &str
parts to String
s efficiently.
Removing the first element afterwards is simple:
args.remove(0);
slice_vec_to_string_vec
can now be removed.
Store an empty vec instead of None
Command.args
is an Option<Vec<String>>
right now, but it could just be a Vec<String>
which is empty to represent None
.
The match cmd.args {
usages will then become if !cmd.args.is_empty() {
.
Prefer taking &str
over String
as argument type
A String
will be automatically coerced into a &str
cheaply, while creating a String
from a &str
is comparatively expensive and must be done manually.
list_all_employees
, add_employee
, del_employee
can all be modified to accept a &str
, such that calling them changes from:
del_employee(table, String::from(&cmd.args[0][..]), String::from(&cmd.args[2][..]));
to:
del_employee(table, &cmd.args[0], &cmd.args[2]);
Print errors with eprintln!
instead of println!
println!
only prints the message to the standard output stream, while eprintln!
prints the message to the standard error stream.
Use longer variable names
Spell emp
and dep
out as employee
and department
. Also, you are not using pointers, as d_ptr
, e_ptr
, ptr
and your error messages suggest, but rather references. Actual pointers in Rust (*const Type
and *mut Type
) are rarely used since they may point to invalid data and dereferencing them is unsafe.
let-else
in main loop
The following destructuring assignment:
let cmd: String = match user_prompt() {
Ok(val) => val,
_ => {
eprintln!("Bad user input. Please try again...");
continue;
}
};
can be expressed more concisely using let-else syntax:
let Ok(cmd) = user_prompt() else {
eprintln!("Bad user input. Please try again...");
continue;
};
The else
branch must always diverge (meaning return
, break
, continue
, panicking or ending the program otherwise) for this syntax to be usable.
Final code
// valid commands:
// ADD name TO department, DEL name FROM department
// LIST department, LIST, QUIT
use std::{collections::HashMap, io, io::Write};
#[derive(Debug)]
struct Command {
c_type: CommandType,
args: Vec<String>,
}
#[derive(Debug)]
enum CommandType {
Add,
List,
Del,
Quit,
}
impl CommandType {
fn new(val: &str) -> Option<CommandType> {
match val {
"ADD" => Some(CommandType::Add),
"DEL" => Some(CommandType::Del),
"LIST" => Some(CommandType::List),
"QUIT" => Some(CommandType::Quit),
_ => None,
}
}
}
fn main() {
let mut employer_table: HashMap<String, Vec<String>> = HashMap::new();
employer_table.insert(
String::from("ENGINEERING"),
vec![
String::from("Lewis"),
String::from("Ellen"),
String::from("Alan"),
],
);
employer_table.insert(
String::from("SALES"),
vec![
String::from("Jane"),
String::from("Robert"),
String::from("Penny"),
],
);
loop {
let Ok(input) = user_prompt() else {
eprintln!("Bad user input. Please try again...");
continue;
};
let Some(cmd) = command_parser(input) else {
eprintln!("Invalid command... Please type ADD, DEL, LIST or QUIT.");
continue;
};
// QUIT is the only command that returns false
if !command_executor(cmd, &mut employer_table) {
break;
}
}
}
fn user_prompt() -> io::Result<String> {
let mut buf = String::new();
let stdin = io::stdin();
// This is a little prompt to indicate that a terminal row is typable...
// Copied this guide on SO to get this prompt working:
// https://stackoverflow.com/a/41387232
print!("> ");
io::stdout().flush().unwrap();
stdin.read_line(&mut buf)?;
Ok(String::from(buf.trim())) // Remove trailing CRLF, convert str to String
}
// valid commands:
// ADD name TO department, DEL name FROM department
// LIST department, LIST, QUIT
fn command_parser(c: String) -> Option<Command> {
let mut args: Vec<String> = c.split(' ').map(String::from).collect(); // Convert string to array of words
let arg0 = args[0].to_uppercase();
let c_type = CommandType::new(&arg0)?;
args.remove(0);
if let Some(last) = args.last_mut() {
*last = last.to_uppercase();
}
Some(Command { c_type, args })
}
fn command_executor(cmd: Command, table: &mut HashMap<String, Vec<String>>) -> bool {
match cmd.c_type {
CommandType::Add => {
if !cmd.args.is_empty() {
add_employee(table, &cmd.args[0], &cmd.args[2])
} else {
eprintln!("Invalid arguments...")
}
true
}
CommandType::List => {
if !cmd.args.is_empty() {
list_all_employees(table, Some(&cmd.args[0]))
} else {
list_all_employees(table, None)
}
true
}
CommandType::Del => {
if !cmd.args.is_empty() {
del_employee(table, &cmd.args[0], &cmd.args[2])
} else {
eprintln!("Invalid arguments...")
}
true
}
CommandType::Quit => {
println!("Thank you for using this shitty system. Goodbye!");
false
}
}
}
fn list_all_employees(table: &HashMap<String, Vec<String>>, opts: Option<&str>) {
match opts {
Some(opt) => match table.get(opt) {
Some(values) => println!("\n{}: {:#?}", opt, values),
None => eprintln!("{} is an invalid department.", opt),
},
None => println!("{:#?}", table),
}
}
fn add_employee(table: &mut HashMap<String, Vec<String>>, employee: &str, department: &str) {
match table.get_mut(department) {
Some(department_ref) => department_ref.push(employee.to_string()),
None => eprintln!(
"Failed to get a reference to the department: {} using employee: {}",
department, employee
),
};
}
fn del_employee(table: &mut HashMap<String, Vec<String>>, employee: &str, department: &str) {
match table.get_mut(department) {
Some(department_ref) => match department_ref.iter().position(|e| *e == employee) {
Some(employee_index) => {
department_ref.remove(employee_index); // Has to have semicolon so that the arm doesn't return anything
}
None => eprintln!("Unable to remove that element..."),
},
None => eprintln!(
"Failed to get a reference to department: {} using employee: {}",
department, employee
),
};
}
Further possible improvements
Missing indexing check
Entering Add Sally
or Del Robert
should give an explanatory error message about the missing department, but instead panics with index out of bounds: the len is 1 but the index is 2
.
to
is ignored Add Sally to Sales
A word between employee and department must exist, but it can be anything. Consider checking that it is actually to
.