Skip to content

Add support for tuple-structs and unit-structs - #5

Open
zakarumych wants to merge 1 commit into
idanarye:masterfrom
zakarumych:struct-kinds
Open

zakarumych wants to merge 1 commit into
idanarye:masterfrom
zakarumych:struct-kinds

Conversation

@zakarumych

@zakarumych zakarumych commented Aug 20, 2018 •

Copy link
Copy Markdown

subj


This change is Reviewable

@idanarye

Copy link
Copy Markdown
Owner
  1. Please add a test
  2. Why would one even need a builder for a unit type? It doesn't offer completeness because we don't support enums, and it doesn't offer any genericness because we don't have a builder trait.

@zakarumych

zakarumych commented Aug 20, 2018 •

Copy link
Copy Markdown
Author
  1. Ok.
  2. One would want builder for unit type in case this unit type is generated via macro or proc-macro. It can become complete with added enum support.

@idanarye

Copy link
Copy Markdown
Owner

I'm actually kind of worried about enum support. Enum support would mean we could do this:

#[derive(TypedBuilder)]
enum Foo {
    Bar {
        a: i32,
        b: i32,
        c: i32,
    },
    Baz {
        e: i32,
        f: i32,
        g: i32,
    },
}

assert!(Foo::build_bar().a(1).b(2).c(3).build() == Foo::Bar { a: 1, b: 2, c: 3 });
assert!(Foo::build_baz().d(4).e(5).f(6).build() == Foo::Baz { d: 4, e: 5, f: 6 });

But... I consider having complex structs as enum variants an anti-pattern, because then you need to destruct it every time you want to access any of the fields. It's better to use nested types:

#[derive(TypedBuilder)]
struct Bar {
    a: i32,
    b: i32,
    c: i32,
}

#[derive(TypedBuilder)]
struct Baz {
    e: i32,
    f: i32,
    g: i32,
}

#[derive(TypedBuilder)]
enum Foo {
    Bar(Bar),
    Baz(Baz),
}

But then you wouldn't need a builder for Foo, because you would just do:

Foo::Bar(Bar::build().a(1).b(2).c(3).build());
Foo::Baz(Baz::build().d(4).e(5).f(6).build());

Now, it would be nice if we could have Foo::build_bar() and Foo::build_baz() for that style as well - but I don't think this is possible with Rust's current type system and macro system.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants