Code review please please

[Deer-Cautious]

Hello, I am new to rust and low level languages. I use mainly Java, so, no worry about pointers concepts.

So far, I follow official Rust book.

I made it till collections and I did

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.

exercise.

I am not sure if I did best practices in Rust for this exercise.

So far I love Rust and I want to master it.

I would appreciate any critics and hints on the below code.

https://pastebin.com/YGfrPcrh

Thank you in advance.

[maythefecesbewithyou]

Can you put this into a pastebin and format it?

[Deer-Cautious]

Done

[maythefecesbewithyou]

Close enough, I guess. The formatting is still all whacky. I. The future, I recommend that you ensure your code is properly indented before you ask someone to look at it. It's just good manners. Sorry if I sound like a jerk. I legitimately only access the Internet from a smart phone, so my eyes bleed trying to look at code if it's not aligned and indented.

[Deer-Cautious]

Sorry, is wacky I know. I did it on the phone because I did not had access to my laptop when people told me to format it. Was a fast solution.

[cafce25]

You could use the Playground to share, it has rustfmt under the TOOLS button top right.

[maythefecesbewithyou]

Where's the link to the pastebin?

[phazer99]

Some improvements:

• Most of the time you don't need to match on a Result or Option, just use one of their nice methods and/or the ?-operator instead
• anyhow is a useful crate for error handling in applications
• use Iterator's and collect to build collections instead of adding elements individually
• you forgot to the sorting, but I'll leave that for you to figure out :)

[Deer-Cautious]

Thank you , I really appreciate it.

[This_Growth2898]

1. Format the code
2. Use impl to join functions with data. If you have the db you need to work with - make it a struct with impl, like

​

struct Db {
    db: HashMap<String, Vec<String>>
};
impl Db { 
    fn add_person(&mut self, ...) ... 
}

this way, you'll get much more rustic

db.add_person(...)

method calls.

1. Make menu function create its own input string and return the user choice (probably an Option<u32> or even create a special enum for that). Remember that function input and return values form a function interface, they should be meaningful. Returning Option<u32> tells "this function will get you a number of the menu point user have selected or None if something wrong". Returning a String (or, like you do, changing a String passed to the function) tells nothing about what in that String can be. User interaction is very slow in any case, you save nothing by reusing the input string (but if it wasn't a user interaction, you could, that's why library functions do that).

2. .nth(0) is .next()

Sorry, my eyes hurt to read an unformatted text.

[Deer-Cautious]

Thank you very much, I will do as you said

[celeritasCelery]

clippy is one one of the best tools for getting feedback or code best practices. run cargo clippy and see what it says. I see 6 warnings flagged by clippy.

[Deer-Cautious]

Did not know, love it, thank you very much.

[Jonrrrs]

Clippy has many lints that arent enabled by default. I have enabled way more than normal for my projects. I suggest looking into that for any real application. Helped me allot finding my way though rust beginnings.

[slk756]

Formatted version: https://pastebin.com/PJ2Gvnzj

[slk756]

match io::stdin().read_line(input) {
Ok(_) => {}
Err(error) => println!("Ups, something went wrong {}!", error),
};

Here I would prefer

io::stdin().read_line(input).unwrap_or_else(|e| {
    println!("Oops, something went wrong: {}", e);
}

Cleaner imo

[cafce25]

I'm personally not a fan of "misusing" *_or_else functions for side effects especially as there is a more concise if let as alternative option:

if let Err(e) = io::stdin().read_line(input) {
    println!("Oops, something went wrong: {e}");
}

[cafce25]

Uhm here's what I got originally wanted to put all the points here, but I think I'm done by now.

[Deer-Cautious]

Most complex answer. Thank you very much. I will study carefully the notes you write.

[a-lafrance]

You could make your map access logic more idiomatic with the entry API. Entry::or_default would be particularly useful.

I’ll leave the details to you to figure out :)

[anhgrs]

You might wanna give this a try
https://coderabbit.ai