Request for Builder macro review / critique

help
1

Hi, I'm fairly new to Rust, so hoping some experienced people can attack critique my code.

I wrote a builder_macro! that generates a struct and a corresponding builder. I have the following questions regarding best practice which probably makes sense after you've seen the builder macro docs:

  • Is it generally better to return Result<T, E> in the build() method?

    [Builder pattern question] suggests it's not a necessity.
    In terms of usage, it's a ThingBuilder().build() vs ThingBuilder().build().unwrap().
    People can use catch_unwind if build() panics, but it doesn't look like good practice.

  • I have seen #[derive(Builder)], and it white-lists certain meta-attributes

    Is there a reason why only doc, allow, and cfg are copied across? (cc: @colin_kiegel)

    I know I gave up on #[derive(Builder)] when hitting an error when having a lifetime error Box<Trait> field.
    I can't seem to reproduce it though -- I hacked something which happily used the #[derive(Builder)] with a Vec<Box<Trait>>>

  • Any other comments around things I don't know to ask

2

Returning Result is definitely the way to go if you have the potential for error. In 'real' code you usually don't want to unwrap unless you've already proven somewhere else that it'll never fail (or you don't care :wink:) . Typically you'd try! or match the result and handle the failure.

An educated guess on the attribute copies is that those are probably most useful attributes for fields, so the documentation, compiler silences and configuration switches follow to the builder method. Maybe there are attributes that are valid for fields but not for functions, so a white-list is easier to work with?

1 Like
3

@azriel91 The answer of @KodrAus is correct. Some attributes don't make sense on functions but are ok on fields. That's why we start with a whitelist which can be extended whenever needed!

https://github.com/colin-kiegel/rust-derive-builder/pull/14#issuecomment-239625200

You will also find a very good explanation of idiomatic error handling in the book: Error Handling

The short story: Use result and avoid unwrap if possible. :slight_smile:

4

Cool thanks, I'll read those sections, and perhaps extend the derive builder to allow the consuming variant (I need that for my project).