Skip to main content
missing word
Source Link
Dornteufel
  • 800
  • 4
  • 22

to is ignored in Add Sally to Sales

to is ignored Add Sally to Sales

to is ignored in Add Sally to Sales

Source Link
Dornteufel
  • 800
  • 4
  • 22

Your questions

  1. 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.

  2. 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 Strings 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.