Which of these code styles do you find preferable?

First option using mut with constructor in the beginning:

  let mut post_form = PostInsertForm::new(
    data.name.trim().to_string(),
    local_user_view.person.id,
    data.community_id,
  );
  post_form.url = url.map(Into::into);
  post_form.body = body;
  post_form.alt_text = data.alt_text.clone();
  post_form.nsfw = data.nsfw;
  post_form.language_id = language_id;

Second option without mut and constructor at the end:

  let post_form = PostInsertForm {
    url: url.map(Into::into),
    body,
    alt_text: data.alt_text.clone(),
    nsfw: data.nsfw,
    language_id,
    ..PostInsertForm::new(
      data.name.trim().to_string(),
      local_user_view.person.id,
      data.community_id,
    )
  };

You can see the full PR here: https://github.com/LemmyNet/lemmy/pull/5037/files

  • al4s@feddit.org
    link
    fedilink
    arrow-up
    22
    ·
    2 months ago

    Definitely the second one.

    1. It avoids Mut
    2. It makes clear that the initialization is over at the end of of the statement. The first option invites people to change some more properties hundreds of lines down where you won’t see them.
  • BB_C@programming.dev
    link
    fedilink
    arrow-up
    20
    ·
    2 months ago

    Neither.

    • make new() give you a fully valid and usable struct value.
    • or use a builder (you can call it something else like Partial/Incomplete/whatever) struct so you can’t accidentally do anything without a fully initialized value.

    Maybe you should also use substructs that hold some of the info.

    • Dessalines@lemmy.ml
      link
      fedilink
      arrow-up
      1
      ·
      edit-2
      2 months ago

      We used to have TypedBuilder (which is builder pattern), but switched to DeriveNew, as its a bit cleaner, and requires less generated code.

  • Deebster@programming.dev
    link
    fedilink
    English
    arrow-up
    12
    ·
    2 months ago

    100% the second one. It’s the idiomatic way to do this in Rust, and it leaves you with an immutable object.

    I personally like to move the short declarations together (i.e. body down with language_id (or both at the top)) but that’s a minor quibble.

  • asudox@programming.dev
    link
    fedilink
    arrow-up
    8
    ·
    edit-2
    2 months ago

    Second one if a constructor or a builder is not an option. 1 is out of the question.

    Why are the Lemmy devs asking for this though?

    • al4s@feddit.org
      link
      fedilink
      arrow-up
      12
      ·
      2 months ago

      If you’re ever forced to do something the second way, you can also wrap it in braces, that way you end up with an immutable value again:

      let app = {
        let mut app = ...
        ...
        app
      };
      
  • livingcoder@programming.dev
    link
    fedilink
    arrow-up
    2
    ·
    edit-2
    1 month ago

    I prefer to encapsulate a mutable reference to the instance in a scope.

    let post_form = {
        let mut post_form = PostInsertForm::new(
            // your constructor arguments
        );
        post_form.some_mutating_method(
            // mutation arguments
        );
        post_form
    };
    

    This way you’re left with an immutable instance and you encapsulate all of the logic needed to setup the instance in one place.

      • livingcoder@programming.dev
        link
        fedilink
        arrow-up
        1
        ·
        edit-2
        1 month ago

        Even if you were using the builder pattern, this maintains the immutable variable in the parent scope while you use the mutable variable’s builder pattern methods (basically exactly as my example demonstrates) in the inner scope.

        edit: Oh, I think you mean you would chain the builder pattern calls and assign it to an immutable variable. Sure, that makes sense if you own the struct.