Starting to learn rust - review request


I am new to rust (and this forum) and already landed on lot on useful information here via search! Thanks to all who put in the effort. :slight_smile:

A small code-snippet that does not do anything meaningful - operates on struct and vectors to understand ownership and borrowing. Requesting members to comment on the code from various perspectives esp. if it's canonical way to to do the thing, performance & safety implications and how to improve the same.

Here is the code:

Hi, welcome! You should run clippy on your code, you can try it in the playground via the tools drop down menu.

1 Like

A couple of comments:

  • Cloning is best done via the Clone trait for interoperability. Even if you are trying to manually implement cloning yourself for learning purposes, impl Clone for Person instead of adding an inherent method directly to the Person type.
  • If you don't need custom cloning behavior, you can just #[derive(Clone)] too.
  • pub fn suffix_first_name(persons: &Vec<Person>) – there is no value in taking an immutable reference to a Vec. You should take a slice (&[Person]) instead – this way, even if one doesn't have a vector but an array or another slice (or really, any of the many types that coerce into slices), s/he will be able to pass it to your function.
  • In this particular case, you don't need the explicit deref in (*p).clone() either, p.clone() will do the job thanks to type inference and autoderef. (These are two keywords you can search for to find out what they are.)
  • Instead of explicit mutation, you could in fact use struct update syntax:
    local_vec.push(Person {first: p.first.clone() + "_F", ..p.clone()});
    although depending on how smart the optimizer is, this might clone the first field unnecessarily (in p.clone()), and might look needlessly complicated for one's taste.
  • In Rust, for simple transformations over data structures, it is idiomatic to use iterators instead of explicit loops.
  • You can use the vec![] macro to create vector "literals" rather than creating a mutable vector and manually push()ing onto it.

To summarize, I'd rewrite your code as follows (playground):

#[derive(Debug, Clone)]
pub struct Person {
    first: String,
    last: String,

pub fn suffix_first_name(persons: &[Person]) -> Vec<Person> {
        .map(|mut p| { p.first += "_F"; p })

pub fn main() {
    let vec = vec![
        Person {first: String::from("James"), last: String::from("Croft")},
        Person {first: String::from("Monica"), last: String::from("Flint")},
    println!("{:?}", suffix_first_name(&vec));

Thanks for introducing clippy. Got two valuable hints there. :+1:

Thanks! This is really helpful. :+1:

1 Like

You're welcome :slight_smile: