Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supply required fields as args to builder() #24

Closed
ThomasAlban opened this issue Jul 30, 2024 · 9 comments
Closed

Supply required fields as args to builder() #24

ThomasAlban opened this issue Jul 30, 2024 · 9 comments
Labels
design needed The feature requires more design effort feature request A new feature is requested

Comments

@ThomasAlban
Copy link

ThomasAlban commented Jul 30, 2024

I'd love to have the option to supply required fields of a struct as args to the first method call (basically deriving a new method), leaving optional ones as separate methods you would then be able to call. This would also mean you should't have to call .build() at the end of the chain, because the data structure is valid 'from the get-go'. This would be great, as currently I use derive-new for when I have a simple struct and can't be bothered to write the new function, but having this built into bon would be much nicer as setters for optional methods would be derived too.

A note for the community from the maintainers

Please vote on this issue by adding a 👍 reaction to help the maintainers with prioritizing it. You may add a comment describing your real use case related to this issue for us to better understand the problem domain.

@Veetaha Veetaha added design needed The feature requires more design effort feature request A new feature is requested labels Jul 30, 2024
@Veetaha
Copy link
Contributor

Veetaha commented Jul 30, 2024

I didn't think of having a macro to derive the new() method in bon, but it probably fits the domain. However, I suppose it's mostly a solved problem today. As you mentioned derive-new works fine for this use case. If you'd like to have setters generated for optional fields, then there are proc macros that do that as well. For example, getset is a cool one, although it hasn't been updated for 3 years.

@ThomasAlban
Copy link
Author

Yeah, I agree there have been solutions but I think bon could do it much better - for example, derive-new doesn't support impl Into (or impl IntoIterator) for args. Also, I probably used the wrong word for 'setters', what I really meant was functions that consume self, set a value to something, and return self again, so you can chain them. Here's an example of how everything I'm talking about could work:

#[bon::new]
struct ExampleStruct {
    #[bon(from-iter)]
    field: HashSet<u32>,
    optional: Option<usize>,
    
}

fn main() {
    let example_struct = Example::new([0, 1, 2]).optional(3);
}

I would definitely prefer to use a feature like this built into bon, as then it could use the same 'ecosystem' of features such as impl IntoIteratoe and impl Into

@Veetaha
Copy link
Contributor

Veetaha commented Jul 31, 2024

Note that this has the problem of allowing you to set the value for the same optional field more than once:

Example::new([0, 1, 2]).optional(3).optional(4);

This can happen if two optional fields have the same type and are named almost the same e.g. x: Option<u32>, y: Option<u32>, or firstname: Option<String>, lastname: Option<String>. It can be possible that due to a copy-paste mistake the same optional field is set twice. With bon::builder such error can't happen because it checks for such an error at compile time by changing the builder type state once the optional field was set.

@ThomasAlban
Copy link
Author

Yeah I agree that's a potential drawback, but lots of Rust APIs use this pattern with a 'new' method where you supply required methods, then optional methods you can chain which set or update other fields, and I think it works quite well. Depending on the use case, I think this pattern works better than the builder pattern currently possible with bon, because it's less verbose, not requiring you to name required fields - e.g when you only have one required field it removes the need to call builder() and build()

@ThomasAlban
Copy link
Author

You could also add the option to supply setters which take in &mut self as well as ones which consume self, returning an updated version of self.
The ones which consume self are useful for the initial construction of the object because it allows you to chain methods, but setters which take in &mut self would be more useful for modification of the object after construction. Yes, there are already crates which do this, but having this built into bon would mean having to invoke fewer macros and bring in fewer dependencies to the project. Let me know what you think!

@Veetaha
Copy link
Contributor

Veetaha commented Jul 31, 2024

I think it's reasonable. Eventually, we'll get there

@ThomasAlban
Copy link
Author

Yeah, I definitely understand this is non-trivial to implement and so will be low on the priority list for now!

@cablehead
Copy link

I think this is implemented now!?

@Veetaha
Copy link
Contributor

Veetaha commented Sep 21, 2024

Yes, according to the title, the issue was implemented in #125. While discussing it we determined that the author of the issue had a bit different requirements in mind. I created separate issues for them:

@Veetaha Veetaha closed this as completed Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design needed The feature requires more design effort feature request A new feature is requested
Projects
None yet
Development

No branches or pull requests

3 participants