SecondHalfGames / yakui

yakui is a declarative Rust UI library for games
Apache License 2.0
222 stars 18 forks source link

Builder pattern #133

Open Uriopass opened 6 months ago

Uriopass commented 6 months ago

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:

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

LPGhatguy commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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.