rust-unofficial / patterns

A catalogue of Rust design patterns, anti-patterns and idioms
https://rust-unofficial.github.io/patterns/
Mozilla Public License 2.0
7.98k stars 363 forks source link

Constructor example rewrite #101

Open Amjad50 opened 4 years ago

Amjad50 commented 4 years ago

I'm used to use Self for the class type inside impl

from:

// A Rust vector, see liballoc/vec.rs
pub struct Vec<T> {
    buf: RawVec<T>,
    len: usize,
}

impl<T> Vec<T> {
    // Constructs a new, empty `Vec<T>`.
    // Note this is a static method - no self.
    // This constructor doesn't take any arguments, but some might in order to
    // properly initialise an object
    pub fn new() -> Vec<T> {
        // Create a new Vec with fields properly initialised.
        Vec {
            // Note that here we are calling RawVec's constructor.
            buf: RawVec::new(),
            len: 0,
        }
    }
}

to:

// A Rust vector, see liballoc/vec.rs
pub struct Vec<T> {
    buf: RawVec<T>,
    len: usize,
}

impl<T> Vec<T> {
    // Constructs a new, empty `Vec<T>`.
    // Note this is a static method - no self.
    // This constructor doesn't take any arguments, but some might in order to
    // properly initialise an object
    pub fn new() -> Self {
        // Create a new Vec with fields properly initialised.
        Self {
            // Note that here we are calling RawVec's constructor.
            buf: RawVec::new(),
            len: 0,
        }
    }
}

this makes it easier in case of renaming etc., what do you think?

yuriykulikov commented 3 years ago

Hey all, I am new to Rust I and I am a little bit confused why this is called a constructor and not a factory function. Does it make sense to rename this?

insanitybit commented 3 years ago

A few things.

  1. I agree that this is a factor, but calling it a constructor is probably reasonable too - though the term has some baggage.
  2. Agreed with 'Self'
  3. (why I came to issues) I think there should be a note about the Default trait - if you implement 'new' with no parameters you should also implement Default.
joseluis commented 3 years ago

I personally prefer it like this:

    pub fn new() -> Self {
        // Create a new Vec with fields properly initialised.
        Vec {
            // Note that here we are calling RawVec's constructor.
            buf: RawVec::new(),
            len: 0,
        }
    }

The Self clearly indicates it's a constructor, and the Vec quickly reminds for which type. It helps when you are implementing for multiple related types in the same file.

MarcoIeni commented 3 years ago

Does it make sense to rename this?

Constructor is the name commonly used in rustlang for this from what I see around. It is common in cpp, too.

I think there should be a note about the Default trait

Definitely, PR well accepted!

Anyway, Self was discussed in #61, too. My point is that it might be better to avoid it in book examples in order to make them clearer. However I extensively use it when I code and a PR where you add a note about the use of Self in the constructor page is well accepted if you want. You could also link to #145 once it is done.

pickfire commented 3 years ago

Anyway, Self was discussed in #61, too. My point is that it might be better to avoid it in book examples in order to make them clearer. However I extensively use it when I code and a PR where you add a note about the use of Self in the constructor page is well accepted if you want.

On the other hand, I never use it unless the type is too complex (multiple generics and lifetimes). Using Self makes the rendered doc harder to figure out what is the type (there is only a word Self) and by looking at the code itself you don't get much context, I still prefer it to be more explicit by just typing out the original type.

People coming from another language may think there can only be one constructor, but in rust you can have multiple. I think the correct term is called static method (and the rest being known as instance methods). https://doc.rust-lang.org/rust-by-example/fn/methods.html