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

Allow conversion to builder with the From trait #69

Closed
nicflower opened this issue Aug 27, 2024 · 4 comments
Closed

Allow conversion to builder with the From trait #69

nicflower opened this issue Aug 27, 2024 · 4 comments
Labels
design needed The feature requires more design effort feature request A new feature is requested

Comments

@nicflower
Copy link

nicflower commented Aug 27, 2024

When applying the builder pattern I find that something useful is the conversion from the built type to the builder, for example:

#[derive(Default)]
struct Animal {
    name: String,
    color: String,
    size: String,
}

struct AnimalBuilder {
    animal: Animal
}

impl From<Animal> for AnimalBuilder {
    fn from(animal: Animal) -> Self {
        Self {
            animal,
        }
    }
}

this is useful when I want to edit just 1 or two fields of the objects and leave the others, because it would allow something like this:

let horse = Animal::builder().name("Jon").color("black").size("small").build();
// horse does stuff and turns into something else
let new_horse = horse.into().size("very big").color("brown").build();

is there any way to achieve this behaviour? If not would you be interested in having something like this?

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
Copy link
Contributor

Veetaha commented Aug 27, 2024

Hi, thank you for creating the issue! I see the use case here. What you could do today with bon is that you can reuse a "common set of data" with the variation of the "Shared Partial Builder" pattern with a closure wrapper.

Example:

let builder = || Animal::builder().name("Jon").build();

let horse_1 = builder().color("black").size("small").build();
let horse_2 = builder().color("white").size("medium").build();

This suggestion specifically works around the limitation that you can't overwrite the member once it's set in the builder (more detailed in the example below)


The idea suggested by you about having a From<T> for TBuilder has a flaw in that TBuilder generated by bon specifically restricts the usage of setters to only be called once per member to prevent unintentional overwrites.

Example:

#[bon::builder]
struct Example {
    field: u32
}

let complete_builder = Example::builder().field(42);

// This fails to compile because the field `field` was already set:
complete_builder.field(55);

// Now imagine we have the `From<Example>` to `ExampleBuilder`
// We also need some way to denote a "complete" (total) builder type state
// Where all members were already initialized from the `Example` struct
// Right now that's not possible, since the builder's generics type signature
// is not stable and documented in `bon`. But let's suppose it's already stable
// for the purpose of this example.
let complete_builder: ExampleBuilder<CompleteState> = Example { field: 42 }.into();

// We can't call the setter for `field` because it was already set (from the `Example` struct in From<>` impl)
complete_buillder.field(55);

The builder would need to have update_{member}(&mut self) methods to explicitly update the existing data in the builder without changing the builder's type state. Which is something that I'm planning to add to bon: an ability to mutate the fields that were set in the builder.


Instead, what I think you'd like to have is the ability to generate setter methods for your struct that use builder-like syntax. I think generating setters is something that can be implemented in bon as a separate proc macro.

There are existing crates that generate setters, for example getset (although this one looks unmaintained, it wasn't updated in the last 3 years).

The idea of generating setters was discussed here as well: #24 (comment). It will be cool if bon could do that, because it would yield the setters API that would be consistent with its builder API conventions. One day bon will get there, but my current priority for bon is to get the #[builder] macro feature-complete first.

So once the setters derive feature is available on bon (it's not today), it'll look smth like this:

#[bon::builder(on(String, into))]
#[bon::setters(on(String, into))]
struct Animal {
    name: String,
    color: String,
    size: String,
}

let horse = Animal::builder()
    .name("Jon")
    .color("black")
    .size("small")
    .build();

// Use setters that consume `self`
let mut new_horse = horse
    .set_size("very big")
    .set_color("brown");

// Or you could just mutate the horse with setters that take `&mut self` instead of `self`
new_horse.set_size_mut("tiny");

@Veetaha Veetaha added the feature request A new feature is requested label Aug 27, 2024
@nicflower
Copy link
Author

If I go ahead with the setters pattern you mentioned, would you consider a PR, or is it something you'd prefer to handle later?

@Veetaha
Copy link
Contributor

Veetaha commented Aug 27, 2024

I'd rather do this later. It's going to be a big feature and require a bunch of internal refactoring. Right now, for example, there are several problems with cfg/cfg_attrs support that we have in builder, that I'd like to resolve first before even designing the macro for setters, because it'll have the same concern.

I'm not even settled on the naming patterns yet. Is it set_{member}(self, value), set_{member}_mut(&mut self, value) or with_{member}(self, value), set_{member}(&mut self, value), I'm not sure yet, and more preliminary design work will be required. Studying the experience of getset, including its open issues before establishing the design will be strictly required.

@Veetaha Veetaha added the design needed The feature requires more design effort label Aug 27, 2024
@Veetaha
Copy link
Contributor

Veetaha commented Nov 10, 2024

I'm closing this issue in favour of #147

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

2 participants