this post was submitted on 23 Sep 2024
15 points (89.5% liked)

Rust Programming

8076 readers
1 users here now

founded 5 years ago
MODERATORS
nutomic@lemmy.ml
Joe@lemmy.ml
AgreeableLandscape@lemmy.ml
15
Which code style to initialize structs? (lemmy.ml)
submitted 3 weeks ago by nutomic@lemmy.ml to c/rust@lemmy.ml
23 comments fedilink hide all child comments
 

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

top 23 comments
sorted by: hot top controversial new old
[–] al4s@feddit.org 22 points 3 weeks 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 20 points 3 weeks ago (1 children)

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 1 points 3 weeks ago* (last edited 3 weeks 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 12 points 3 weeks 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.

[–] AsudoxDev@programming.dev 9 points 3 weeks ago* (last edited 3 weeks ago) (1 children)

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?

[–] nutomic@lemmy.ml 2 points 2 weeks ago

To decide if I should merge the linked PR or not (I did merge it).

[–] DemocratPostingSucks@lemm.ee 8 points 3 weeks ago

Defo the second one, the first is weird imo

[–] nutomic@lemmy.ml 5 points 3 weeks ago (1 children)

@DemocratPostingSucks@lemm.ee @Deebster@programming.dev @al4s@feddit.org Thanks for the feedback! Personally I prefer the first option, but based on your comments I will merge the PR with the second option.

[–] al4s@feddit.org 12 points 3 weeks ago (2 children)

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
};
[–] nutomic@lemmy.ml 1 points 3 weeks ago (1 children)

Thats even more verbose so the second option is better.

[–] al4s@feddit.org 6 points 3 weeks ago (1 children)

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

[–] taladar@sh.itjust.works 2 points 3 weeks ago (1 children)

The first one won't work either for private fields.

[–] al4s@feddit.org 2 points 2 weeks ago (1 children)

You can have setters that set private fields, there are also sometimes structs with mixed private and public fields

[–] taladar@sh.itjust.works 1 points 2 weeks ago (1 children)

But why not use a proper builder pattern in that case?

[–] al4s@feddit.org 2 points 2 weeks ago

Because you don't control third party libraries

[–] taladar@sh.itjust.works 1 points 3 weeks ago (1 children)

Why not just a let app = app; line after the let mut app = ...; one?

[–] al4s@feddit.org 2 points 2 weeks ago (2 children)

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 3 points 2 weeks ago

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

[–] taladar@sh.itjust.works 1 points 2 weeks ago

But a scope adds a nesting level which adds a lot more visual clutter.

[–] livingcoder@programming.dev 2 points 2 weeks ago* (last edited 2 weeks ago) (1 children)

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.

[–] ubik@fedi.turbofish.cc 1 points 2 weeks ago (1 children)

@livingcoder @nutomic that's a nice one. Had never thought of it. But I'd just use the builder pattern.

[–] livingcoder@programming.dev 1 points 2 weeks ago* (last edited 2 weeks 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.

[–] Nothing4You@programming.dev 2 points 3 weeks ago

also adding my vote for the second one