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

  • BB_C@programming.dev
    7·
    1 month 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.

  • al4s@feddit.org
    5·
    1 month 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.
    • al4s@feddit.org
      3·
      1 month 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
};
        • al4s@feddit.org
          1·
          1 month ago

          Yeah if you have the second option, use it, but if the struct has private fields it won’t work.

        • al4s@feddit.org
          1·
          1 month ago

          A scope groups the initialization visually together, while adding the let app = app; feels like it just adds clutter - I’d probably just leave it mut in that case.

          • BB_C@programming.dev
            2·
            1 month ago

            Rebinding with and without mut is a known and encouraged pattern in rust. Leaving things as mut longer than necessary is not.

  • Deebster@programming.devEnglish
    2·
    1 month 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.