Repository navigation
Is the Builder pattern an anti-pattern in Rust due to the Default trait? #64
Description
Activity
I'm sympathetic to this, specifically about code size, but have found default alone to fall short in certain cases.
https://github.com/colin-kiegel/rust-derive-builder helps address the extra coding issue but doesn't make the code size issue go away. It just derives it for you at compile time.
Cases were default alone fell short were roughly cases of exposing public apis. In particular ones where I didn't want to expose the internal API ( fields ) of my structs directly as pub fields to consumers outside my crate. Once you expose those it's hard to evolve an API without making breaking changes.
Builders can often help with ergonomics. Methods which accept Into<> arguments are often more flexible and ergonomic for consumers. You lose those when you expose the ability to create a struct directly with it's internal fields.
For point 2, I'm not sure that's an actual problem in practice. Immutible builders prevent that particular case but mutable builders can see benefit allowing users to initialize fields with default values that make sense per application and have it double initialized with an override for target cases. With default, you take that control out of the users hands. They can not implement a trait like default that's defined in another crate
For point 3 it's actually useful to have a method like build to validate the semantic correctness of fields to ensure structs may only be created with those semantics.
I think it's fair to say that default impls and builders complement each other. It's often useful to use both in some combination. The derive builder crate supports this. In specific cases you may only need one or the other. I don't think either in isolation in all cases is a silver bullet for struct creation.
Reacted by Jeff Walker, zzeroo, Jesse Wright, Andrew Gaspar, Paul-Sebastian Manole, Harrison McCullough, Marcin Pajkowski, Odin, aheart, Dimitris Giannakos and 2 moreImplementing
Defaultmight not be feasible if there is no sensible default for at least one field.The builder pattern doesn't protect against double-initialization
You could protect against this by tracking the initialization of individual fields at the type level, but then adding a new field to the builder becomes a breaking change, which defeats one of the purposes of builders.
Reacted by Michael Cooper, anderejd, Daniel Lubarov, aheart, I60R and Vic Shihang LiI think your
WindowBuildercannot be considered an example of the builder pattern, just senseless code repetition. It does not "separate construction from representation" since the same representation works.You need some essential transition in a builder pattern that requires changing the representation, normally a data structure's parts must be transformed simultaneously. As a rule, builder outputs are more opaque than their inputs too. This is a builder pattern:
pub struct WindowBuilder { pub title: &'static str, pub width: usize, pub height: usize, } impl Default for WindowBuilder { fn default() -> Self { ... } } impl WindowBuilder { pub fn draw(self) -> OpaqueWindowHandle { ... } }What if
OpaqueWindowHandleis simplystruct Opaque(WindowBuilder)? Isn't that silly too but now an honest transition? In practice, you hide data to avoid inconsistencies with other data, so thedrawmethod likely adds or transforms data in a way that permits conflicts withWindowBuilder.As an aside, there were interesting discussions about private data handling in functional record updates. I think say
TmpMmapBuilder { size: 100, ..Default::default() }.make()should work withpub struct TmpMmapBuilder<T> { path: Path, // not pub pub size: usize, pub whatever: Whatever, PhandomData<[T]>, } impl Default for TmpMmapBuilder { .. } impl TmpMmapBuilder<T> { fn make() -> Mmap { ... } }Reacted by Sova, Svetlin Zarev, Mihail Malo, Günter Zöchbauer, Adam Batkin and AlephAlphaI've never used it but maybe the
derive_buildercrate encourages some anti-pattern here. At leastderive_builderwithout any tweaks looks questionable.As an aside, I think many builder patterns take some parameters struct with
pubelements for all or part of the builder and package them up as a hidden element of the built object, likepub struct ObjectParams { pub foo: Foo, // pub so user can change them pub bar: Bar } pub struct Object { params: ObjectParams, // No longer pub so user cannot change anything once built ... // Stuff that actually gets built } impl Default for ObjectParams { .. } impl ObjectParams { fn build(self, ...) -> Object { ... } }This style will become more flexible once Rust adds nameless struct items, so maybe
pub struct ObjectBuilder { pub _: ObjectParams, // Makes ObjectBuilder expose all ObjectParams' elements directly meow: Meow, // manipulated only indirectly by builder methods }Reacted by Mark LeMoineYou've got an error in your code:
pub fn build(self) -> Window { Window { title: self.title, width: self.width, height: self.height, } }
Should have been:
pub fn build(self) -> Window { Window { title: self.__title, width: self.__width, height: self.__height, } }
Sorry to step in. I was looking at the builder pattern and got here. I have a proposal I just used in some code and I wanted to share, even though it may turn out to be a terrible idea (and feel free to say so).
In a nutshell, I used the
Fromtrait:// A struct I want to have built pub struct MyStruct { // A private field secret: u32, } // Builder for MyStruct pub struct MyStructBuilder { // A public field pub open_data: u32, } impl From<MyStructBuilder> for MyStruct { fn from(builder: MyStructBuilder) -> Self { MyStruct { secret: builder.open_data, } } } fn main() { let builder = MyStructBuilder { open_data: 5 }; let my_struct = MyStruct::from(builder); }
MyStructBuildermay implement theDefaulttrait, so we can easily induce that onMyStructas well:impl Default for MyStruct { fn default() -> Self { MyStruct::from(MyStructBuilder::default()) } }
If
MyTraitBuilder::fromcan fail, theFromtrait could be implemented forResult<MyStruct, ErrorType>instead:// A struct I want to have built pub struct MyStruct { // A private field secret: u32, } // Builder for MyStruct pub struct MyStructBuilder { // A public field pub open_data: i32, } impl From<MyStructBuilder> for Result<MyStruct, ()> { fn from(builder: MyStructBuilder) -> Self { if builder.open_data >= 0 { Ok(MyStruct { secret: builder.open_data as u32, }) } else { Err(()) } } }
To justify my approach: the point is that, by using public fields in
MyStructBuilder, I don't have to write methods to set each field. Still, the fields inMyStructare private, so onlyMyStructcan set them. I could write allset_fieldmethods forMyStruct, but maybe I don't want to. The solution is to use theFromtrait, so that I can access private fields directly.Reacted by Artur Sinila and pyk- Reacted by Sidney Liebrand and Gerald Davis
Perhaps the builder pattern doc should mention that this is about initializing private struct fields.
There is no point in using builder (or multiple constructors which the builder pattern is supposed to help to avoid) if all fields are public.Reacted by nuxehI've done this a couple times:
pub struct WindowBuilder { pub title: &'static str, pub width: usize, pub height: usize, } pub struct Window { builder: WindowBuilder, // Builder here avoids field repitition, but its no longer public. derived_data: .., }It's becomes tricky whether you want
Window: Deref<Target=WindowBuilder>withoutDerefMutinstead of getter methods though, so users loose access toselfor&mut selfmethods onWindowBuilder- addedC-needs discussionArea: Something that is not clear to everyone if it fixes something/adds valuable contentArea: Something that is not clear to everyone if it fixes something/adds valuable content
on Dec 31, 2020 - changed the title
[-]Builder pattern is an anti-pattern[/-][+]Is the `Builder` pattern an anti-pattern in Rust due to the `Default` trait?[/+]on Jan 21, 2021 - addedM-move-to-discussionsMeta: Label for converting issues to discussionsMeta: Label for converting issues to discussionsand removedC-needs discussionArea: Something that is not clear to everyone if it fixes something/adds valuable contentArea: Something that is not clear to everyone if it fixes something/adds valuable content
on Feb 22, 2021 - locked and limited conversation to collaborators
on Feb 22, 2021
I claim that the builder pattern is more or less an anti-pattern and that you should use the
Defaulttrait instead. Here's why:Let's say we have a struct:
Creator:
See how much code we need to construct a window in comparison to the
Defaulttrait?User:
The
Defaulttrait protects against that, because you can't initialize the same field twice. The builder pattern simply overwrites the field and you don't get any warning.Defaulttrait eliminates the need for theSomethingBuilderstruct. TheSomethingBuilderstruct is an intermediate struct that provides a certain kind of type safety so that you have to callSomethingBuilder.build()to construct aSomethingout of aSomethingBuilder. All of this is unnecessary if you use theDefaulttrait - less code with essentially the same outcome. TheSomethingBuilderhas one appropriate use, in my opinion: When you need something to happen in the.build()function and it needs to happen once (although you can implement this in adefault()function, too). For example, you need to tell the OS to create a window. This is where it's appropriate to use a builder. However, I've seen the builder pattern to be completely overused, which is why I'm writing this.Often times when I'm having a struct with many fields that can have default values, it is easier to implement a default trait than to write ten or twenty builder functions. And that is why I claim that the builder pattern is actually an anti-pattern and that you should use the
Defaulttrait instead, wherever possible. It should at least be included somewhere in this repository.