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

Builder pattern #133

Open
Uriopass opened this issue Dec 23, 2023 · 4 comments
Open

Builder pattern #133

Uriopass opened this issue Dec 23, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@Uriopass
Copy link
Contributor

The recommended (cf examples) way to pass properties to widgets today is to do this:

let mut l = List::row();
l.main_axis_alignment = ...;
l.show_children(|| { ... });

I feel the temporary variable is not as ergonomic compared to the usual builder pattern:

List::row()
   .main_axis_alignment(...)
   .show_children(|| { ... });

The builder pattern is quite useful when one needs to migrate from short-hands i.e row(|| .. to List::row().thing(..).show(|| )
as you can just edit the line and rustfmt will automatically re-separate it for you.

I do see a couple of issues with the builder pattern:

  • Compile times (not sure)
  • Auto-completion is weird as all fields of the builder are public (part of yakui's philosophy) so both the field and the builder function appear
  • Leads to more indentation which is already a scarce resource

I'm guessing the last reason might be why it was chosen, but I might have missed something?

@LPGhatguy LPGhatguy added the enhancement New feature or request label Dec 24, 2023
@LPGhatguy
Copy link
Member

This originally happened as an experiment to see if it would be ergonomic enough to just have public fields on #[non_exhaustive] structs. It's kind of a pain though, and I think your example of replacing short-hands is great rationale for adding builders.

One other benefit of builders is that it enables us to use traits to make some conversions more ergonomic. That comes up a lot when a widget wants a String or Cow<str> but the user has a &str.

Do you know of any crates that might help us cut down on duplicate documentation between the builder and any public fields? Maybe that's something we could delegate to a macro_rules macro, but I worry about making widget code more opaque.

@Uriopass
Copy link
Contributor Author

One other benefit of builders is that it enables us to use traits to make some conversions more ergonomic. That comes up a lot when a widget wants a String or Cow but the user has a &str.

That's a great point!

Do you know of any crates that might help us cut down on duplicate documentation between the builder and any public fields? Maybe that's something we could delegate to a macro_rules macro, but I worry about making widget code more opaque.

All the builder pattern generation crate seem to create a secondary "FooBuilder" type that doesn't really align with what we want.
I agree that a macro_rules! would be annoying. The simplest way might just be to duplicate the doc by hand or not document the fields at all.

I'd still like to expand on the indentation problem:

    let mut l = List::row();
    l.main_axis_alignment = MainAxisAlignment::Center;
    l.show(|| {
        let mut l2 = List::row();
        l2.main_axis_alignment = MainAxisAlignment::Center;
        l2.show(|| {
            let mut l3 = List::column();
            l3.main_axis_alignment = MainAxisAlignment::Center;
            l3.show(|| {});
        });
    });

    // same code using builder pattern
    List::row()
        .main_axis_alignment(MainAxisAlignment::Center)
        .show(|| {
            List::row()
                .main_axis_alignment(MainAxisAlignment::Center)
                .show(|| {
                    List::row()
                        .main_axis_alignment(MainAxisAlignment::Center)
                        .show(|| {});
                });
        });

Although from my limited experience, most of the gui code is actually short hands so that might not be that big of a problem.

In the end, I'm really on the fence about the builder pattern.

@kanerogers
Copy link
Collaborator

Personally I'm keen on builders. I almost always end up overriding the default shorthands, and while it's no big deal to initialise and mutate, I think a builder would feel a bit more natural.

@LPGhatguy
Copy link
Member

One mark against the variable+mutation pattern that yakui has today is that you have to come up with a name for the binding. When you have a few of those in scope, it's a little yucky to read.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants