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

option to completely customise input type of builder methods #22

Open
ThomasAlban opened this issue Jul 30, 2024 · 6 comments
Open

option to completely customise input type of builder methods #22

ThomasAlban opened this issue Jul 30, 2024 · 6 comments
Labels
feature request A new feature is requested

Comments

@ThomasAlban
Copy link

ThomasAlban commented Jul 30, 2024

I'd love to be able to completely customise the input type for builder methods, for situations where impl Into isn't enough.
For example, if I have a HashSet<T> as a struct field, it would be much nicer to accept an impl IntoIterator<Item = T> then have the buider method do input.into_iter().collect() rather than the builder method accepting impl Into<HashSet<T>> and the user having to do the rest.
Not sure how this could be implemented, there would have to be a way to specify the desired input type, and also perhaps pass in to the macro a function that goes from the input type to the type of the struct field. E.g:

#[builder]
struct ExampleStruct {
    #[builder(input_type = impl IntoIterator<Item = u32>, convert_fn = convert)]
    example: HashSet<u32>,
}

fn convert(input: impl IntoIterator<Item = u32>) -> HashSet<u32> {
    input.into_iter().collect()
}

Or perhaps it could be possible to supply a closure:

#[builder]
struct ExampleStruct {
    #[builder(input_type = impl IntoIterator<Item = u32>, convert_fn = |x| x.into_iter().collect())]
    example: HashSet<u32>,
}

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 the enhancement New feature or request label Jul 30, 2024
@Veetaha
Copy link
Contributor

Veetaha commented Jul 30, 2024

Thank you for bringing this up! This is something I was also thinking about. For example typed-builder provides #[builder(setter(transform = |x: f32, y: f32| Point { x, y }))] attribute to customize the setter. It expects a closure with type annotations to do this. This is entirely possible to implement in bon as well.

I didn't do this initially to keep the initial release simple and to keep the number of attributes and magic as low as possible. For example, today you can implement this by defining your builder via a new() method instead:

use bon::bon;

struct ExampleStruct {
    example: HashSet<u32>,
}

#[bon]
impl ExampleStruct {
    #[builder]
    fn new(example: impl IntoIterator<Item = u32>) -> Self {
        Self { example: <_>::from_iter(example) } 
    }
}

I understand that this requires a bit more boilerplate, however, this makes the code simpler. The fallback to a #[builder] on fn new basically solves any requirements for custom logic overrides via attributes.

On the other hand, I suppose you'd like to have a general capability to accept impl IntoIterator for collections, right? This is something that can be special-cased with #[builder(from_iter)], because I think this may be a frequent use case.

I'd prefer a #[builder(from_iter)] in this case and recommend falling back to an explicit #[builder] fn new() method for any other advanced use cases.

What do you think? Will #[builder(from_iter)] be enough for your observable use cases?

@Veetaha Veetaha added feature request A new feature is requested and removed enhancement New feature or request labels Jul 30, 2024
@ThomasAlban
Copy link
Author

Yeah, for me at least #[builder(from_iter)] would be fine! Not sure if you should name it differently though, given from_iter implies impl Iterator when it's really impl IntoIterator... your choice!
In the long run, having something like a closure would be nice, but for now this would solve everything I want to do :)

@Veetaha
Copy link
Contributor

Veetaha commented Jul 30, 2024

given from_iter implies impl Iterator when it's really impl IntoIterator

I agree. It's unfortunate this naming choice was established in Rust FromIterator trait that actually powers the .collect() method. Even though the trait and its method imply impl Iterator, it accepts impl IntoIterator instead. On the other hand I guess #[builder(into_iter)] may read better

@ThomasAlban
Copy link
Author

ThomasAlban commented Jul 30, 2024

Yeah it's kind of weird but if that's the norm in rust then it definitely makes sense!
Anyway thanks a lot for responding to this so fast, I've been having a lot of fun refactoring parts of my project to use bon (saved loads of lines of code!) - I discovered it off your reddit post and I don't think I'll be able to live without it from now on! I do have one or two other suggestions for features but I guess I'll open other issues for those.

@Veetaha
Copy link
Contributor

Veetaha commented Jul 30, 2024

Glad to hear that! I noticed that you added bon as a git dependency to your project. Is there any reason not to use it from crates.io?

@ThomasAlban
Copy link
Author

Oh yeah, that was from when you had fixed the issue about optional types having to implement default on main, but before you had published a new crates.io release. I have changed it just not committed those changes to gh yet!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request A new feature is requested
Projects
None yet
Development

No branches or pull requests

2 participants