Builder pattern in Rust

Question

I tried to implement Builder pattern, a popular design pattern for constructing objects consisting of different components, in Rust.

Here is my attempt:

#[derive(Debug)]
enum WheelConfiguration {
 TwoWheeler,
 ThreeWheeler,
 FourWheeler,
 FourPlusWheeler,
}
#[derive(Debug)]
enum EngineConfiguration {
 SmallEngine,
 MediumEngine,
 LargeEngine,
}
#[derive(Debug)]
enum SeatConfiguration {
 TwoSeater,
 FourSeater,
 FiveSeater,
}
#[derive(Debug)]
struct Vehicle {
 wheels: WheelConfiguration,
 engine: EngineConfiguration,
 seats: SeatConfiguration,
}
struct VehicleBuilder {
 vehicle: Vehicle,
}
impl VehicleBuilder {
 fn new() -> Self {
 Self {
 vehicle: Vehicle {
 wheels: WheelConfiguration::TwoWheeler,
 engine: EngineConfiguration::SmallEngine,
 seats: SeatConfiguration::TwoSeater,
 }
 }
 }
 
 fn with_wheels(&mut self, wheels: u32) -> &mut VehicleBuilder {
 self.vehicle.wheels = match wheels {
 1 => {
 eprintln!("Invalid wheel configuration: {}", wheels);
 return self;
 }
 2 => WheelConfiguration::TwoWheeler,
 3 => WheelConfiguration::ThreeWheeler,
 4 => WheelConfiguration::FourWheeler,
 _ => WheelConfiguration::FourPlusWheeler,
 };
 return self;
 }
 
 fn with_engine(&mut self, engine: &str) -> &mut VehicleBuilder {
 self.vehicle.engine = match engine {
 "small" => EngineConfiguration::SmallEngine,
 "medium" => EngineConfiguration::MediumEngine,
 "large" => EngineConfiguration::LargeEngine,
 _ => {
 eprintln!("Invalid engine configuration: {}", engine);
 return self;
 }
 };
 return self;
 }
 
 fn with_seats(&mut self, seats: u32) -> &mut VehicleBuilder {
 self.vehicle.seats = match seats {
 2 => SeatConfiguration::TwoSeater,
 4 => SeatConfiguration::FourSeater,
 5 => SeatConfiguration::FiveSeater,
 _ => {
 eprintln!("Invalid seat configuration: {}", seats);
 return self;
 }
 };
 return self;
 }
 
 fn finalize_build(&self) -> &Vehicle {
 &self.vehicle
 }
}
fn main() {
 let mut builder = VehicleBuilder::new();
 let vehicle_ref = builder
 .with_wheels(1)
 .with_engine("small")
 .with_seats(4)
 .finalize_build();
 println!("vehicle is {:?}", vehicle_ref);
}

Please review this code and let me know if something can be improved.

• Thibe: I noticed a small unhandled check where 0-wheels are treated as FourPlusWheeler.
• kiner_shah: @Thibe, nice catch! :-)
• aghast: Shouldn't FourPlusWheeler simply be called MoreWheeler? ;->
• kiner_shah: @aghast, yes that also works.
• mhovd: I would suggest making the enums public, and allowing the user to choose that directly, rather than parsing usize and &str.

Answer 1

Ownership

Builder should return an instance, not a reference to it, and be consumed during build call. This is semantically correct: builder is a one-off logic container with sole purpose of creating some object. Normally all its methods take ownership and return self, like this (with blocks replaced by unreachable to avoid copying a wall of code):

impl VehicleBuilder {
 fn new() -> Self {
 unreachable!();
 }
 
 fn with_wheels(mut self, wheels: u32) -> Self {
 unreachable!();
 }
 
 fn with_engine(mut self, engine: &str) -> Self {
 unreachable!();
 }
 
 fn with_seats(mut self, seats: u32) -> Self {
 unreachable!();
 }
 
 fn finalize_build(self) -> Vehicle {
 self.vehicle
 }
}

This is to avoid producing a reference. The final user can make a reference if needed, but going other way requires wasteful cloning.

You can use a builder example in the unofficial rust book for reference.

Model definition

In your domain model, is there a reasonable definition of a default vehicle configuration? Same question about every separate enum.

• If yes: move the default vehicle to Default trait (or #[derive(Default)]) for better visibility of what the "default" looks like.
• If no: do not assume a default vehicle configuration, make all builder steps required. That probably means duplicating Vehicle fields and wrapping them in Option.

I assume enums are just placeholders, but if not - derive at least Clone and Copy on them, plain enums without content are cheap, this would be a great quality of life improvement.

If defaults make sense, that would be

#[derive(Debug, Default)]
enum WheelConfiguration {
 #[default]
 TwoWheeler,
 ThreeWheeler,
 FourWheeler,
 FourPlusWheeler,
}
#[derive(Debug, Default)]
enum EngineConfiguration {
 #[default]
 SmallEngine,
 MediumEngine,
 LargeEngine,
}
#[derive(Debug, Default)]
enum SeatConfiguration {
 #[default]
 TwoSeater,
 FourSeater,
 FiveSeater,
}
#[derive(Debug, Default)]
struct Vehicle {
 wheels: WheelConfiguration,
 engine: EngineConfiguration,
 seats: SeatConfiguration,
}
// ....
impl<'a> VehicleBuilder<'a> {
 fn new() -> Self {
 Self {
 vehicle: Vehicle::default(),
 }
 }
}

If not, VehicleBuilder would be essentially a copy of Vehicle with all fields wrapped in Option, and finalize_build will return Err if some fields were not provided. Derive Default on such struct to make new implementation just a wrapper around Self::default(). You can mix&match these two approaches - if some fields have sensible defaults and some do not, you can avoid wrapping the former with Option and just use the default value.

Error handling

Whichever way you go, the canonical way to handle faults in builder is to fail the build. You could make every step fallible, but that's slightly less user-friendly (but manageable with ? unwrapping). For example, like this:

struct VehicleBuilder {
 vehicle: Vehicle,
 errors: Vec<String>,
}
impl VehicleBuilder {
 fn new() -> Self {
 Self {
 vehicle: Vehicle {
 wheels: WheelConfiguration::TwoWheeler,
 engine: EngineConfiguration::SmallEngine,
 seats: SeatConfiguration::TwoSeater,
 },
 errors: vec![],
 }
 }
 
 fn with_wheels(mut self, wheels: u32) -> Self {
 self.vehicle.wheels = match wheels {
 1 => {
 self.errors.push(format!("Invalid wheel configuration: {}", wheels));
 return self;
 }
 2 => WheelConfiguration::TwoWheeler,
 3 => WheelConfiguration::ThreeWheeler,
 4 => WheelConfiguration::FourWheeler,
 _ => WheelConfiguration::FourPlusWheeler,
 };
 self
 }
 
 fn with_engine(mut self, engine: &str) -> Self {
 self.vehicle.engine = match engine {
 "small" => EngineConfiguration::SmallEngine,
 "medium" => EngineConfiguration::MediumEngine,
 "large" => EngineConfiguration::LargeEngine,
 _ => {
 self.errors.push(format!("Invalid engine configuration: {}", engine));
 return self;
 }
 };
 self
 }
 
 fn with_seats(mut self, seats: u32) -> Self {
 self.vehicle.seats = match seats {
 2 => SeatConfiguration::TwoSeater,
 4 => SeatConfiguration::FourSeater,
 5 => SeatConfiguration::FiveSeater,
 _ => {
 self.errors.push(format!("Invalid seat configuration: {}", seats));
 return self;
 }
 };
 self
 }
 
 fn finalize_build(self) -> Result<Vehicle, Vec<String>> {
 if self.errors.is_empty() { 
 Ok(self.vehicle)
 } else {
 Err(self.errors)
 }
 }
}
fn main() {
 let result = VehicleBuilder::new()
 .with_wheels(1)
 .with_engine("small")
 .with_seats(4)
 .finalize_build();
 match result {
 Ok(vehicle) => println!("vehicle is {:?}", vehicle),
 Err(errors) => {
 eprintln!("Invalid confugration! The following problems were detected:");
 for err in errors {
 eprintln!("* {err}");
 }
 }
 };
}

This way, the caller knows when something goes wrong. You can mix and match - for example, only store an Option<String> and report the first/last error. I'm sticking to strings to show the idea, in practice that would probably be

use thiserror::Error;
struct VehicleBuilder<'a> {
 vehicle: Vehicle,
 errors: Vec<VehicleBuilderError<'a>>,
}
#[derive(Error, Debug)]
pub enum VehicleBuilderError<'a> {
 #[error("Invalid wheel configuration: '{0}'")]
 InvalidWheel(u32),
 #[error("Invalid engine configuration: '{0}'")]
 InvalidEngine(&'a str),
 #[error("Invalid seat configuration: '{0}'")]
 InvalidSeat(u32),
}
impl<'a> VehicleBuilder<'a> {
 fn new() -> Self {
 ...
 }
 
 fn with_wheels(mut self, wheels: u32) -> Self {
 self.vehicle.wheels = match wheels {
 1 => {
 self.errors.push(VehicleBuilderError::InvalidWheel(wheels));
 return self;
 }
 2 => WheelConfiguration::TwoWheeler,
 3 => WheelConfiguration::ThreeWheeler,
 4 => WheelConfiguration::FourWheeler,
 _ => WheelConfiguration::FourPlusWheeler,
 };
 return self;
 }
 
 fn with_engine(mut self, engine: &'a str) -> Self {
 self.vehicle.engine = match engine {
 "small" => EngineConfiguration::SmallEngine,
 "medium" => EngineConfiguration::MediumEngine,
 "large" => EngineConfiguration::LargeEngine,
 _ => {
 self.errors.push(VehicleBuilderError::InvalidEngine(engine));
 return self;
 }
 };
 return self;
 }
 
 fn with_seats(mut self, seats: u32) -> Self {
 ...
 }
 
 fn finalize_build(self) -> Result<Vehicle, Vec<VehicleBuilderError<'a>>> {
 if self.errors.is_empty() { 
 Ok(self.vehicle)
 } else {
 Err(self.errors)
 }
 }
}

Trailing return

Blocks can have a trailing value in Rust. Conventionally, functions that return something at the end omit the return:

fn something(self) -> Self {
 self
}

Simpler

This is fine:

println!("vehicle is {:?}", vehicle)

but this is slightly more readable, especially when there are several variables:

println!("vehicle is {vehicle:?}")

Accessibility

I'd add a builder associated function to Vehicle to make the builder easier to discover:

impl Vehicle {
 pub fn build() -> VehicleBuilder {
 VehicleBuilder::new()
 }
}

Shift of responsibility

What you're doing might be reasonable in some cases. However, if the parsing makes sense for every configuration block as a separate entity, consider offloading the parsing work to it.

In other words, do you want to allow creating WheelConfiguration from a number?

• If no, consider requiring WheelConfiguration in builder too. But then the whole builder becomes pointless, you can just move parsing to the enums, remove builder and create Vehicle as a struct literal.
• If yes, consider implementing TryFrom on the enum (and add a enum variant to stop discriminating monowheels!):

use thiserror::Error;
#[derive(Debug)]
enum WheelConfiguration {
 TwoWheeler,
 ThreeWheeler,
 FourWheeler,
 FourPlusWheeler,
}
#[derive(Error, Debug)]
#[error("Invalid wheel: '{0}'")]
struct InvalidWheel(u32);
impl TryFrom<u32> for WheelConfiguration {
 type Error = InvalidWheel;
 fn try_from(wheels: u32) -> Result<Self, Self::Error> {
 match wheels {
 0 | 1 => Err(InvalidWheel(wheels)),
 2 => Ok(Self::TwoWheeler),
 3 => Ok(Self::ThreeWheeler),
 4 => Ok(Self::FourWheeler),
 _ => Ok(Self::FourPlusWheeler),
 }
 }
}
// and now
type BuilderErrors<'a> = Vec<Box<dyn std::error::Error + 'a>>;
struct VehicleBuilder<'a> {
 vehicle: Vehicle,
 errors: BuilderErrors<'a>,
}
impl<'a> VehicleBuilder<'a> {
 fn new() -> Self {
 Self {
 vehicle: Vehicle {
 wheels: WheelConfiguration::TwoWheeler,
 engine: EngineConfiguration::SmallEngine,
 seats: SeatConfiguration::TwoSeater,
 },
 errors: vec![],
 }
 }
 
 fn with_wheels(mut self, wheels: u32) -> Self {
 match wheels.try_into() {
 Ok(parsed) => self.vehicle.wheels = parsed,
 Err(e) => self.errors.push(Box::new(e)),
 }
 self
 }
 // ...
 
 fn finalize_build(self) -> Result<Vehicle, BuilderErrors<'a>> {
 if self.errors.is_empty() { 
 Ok(self.vehicle)
 } else {
 Err(self.errors)
 }
 }
}

Note that it still might make sense to remove the builder after this and use literal construction, as now the builder just accumulates errors without any other logic.

• Tvde1: This allows vehicle.build().finalize_build().build().finalize_build()... :)
• kiner_shah: So you have used mut self instead of &mut self that will move the instance of builder across functions and then after finalize_build the instance will be lost. Maybe it can be made to accept &mut self and then the instance won't be lost, but can rather be reused to construct more vehicle objects (will probably have to reset vehicle in finalize_build before return to default values). What do you think? Although, it would probably require cloning of vehicle member.
• STerliakov: @Tvde1 would be fun, but no, build is an associated function, it can't be called on instances:(
• STerliakov: @kiner_shah feels like it won't pass borrow checker. If you want to raise the builder after some steps, just make it Clone. Accepting &mut self won't make it reusable: you cannot have two mutable references to the same builder, so cloning is necessary anyway.

Answer 2

Builders should only construct the object at build time

Your VehicleBuilder's constructor already creates a Vehicle, but that is something you should avoid. It is better to only store the desired properties as member variables, like the wheel, engine and seat configuration. Then only when calling build() should you construct an actual Vehicle object and return it.

There are several advantages doing it that way:

• Possibly less memory usage (in case the type of object you build is much larger than just the parameters the builder allows to change)
• Avoids needing the type of object you want to build to be copyable.
• Avoids the problem of what default parameters to use, in case there is no default constructor for the type of object you want to build.

It would also have avoided the mistake of returning a reference.

• STerliakov: Avoids needing the type of object you want to build to be copyable - wait, but Vehicle here isn't Clone nor Copy, so it isn't, and yet the code compiles? I'd say that using a Default object as a placeholder is reasonable as long as a reasonable default can be defined (and only duplicate properties otherwise). You can see such builders wrapping an already constructed instance as "configurators" - wrappers to override the default behaviour. (but this is indeed wasteful if Default impl is non-trivial)
• G. Sliepen: @STerliakov Indeed, in OP's code it just works because they return a reference, although that introduces a lifetime issue if you want the object to outlive its builder. In fact, many of the GoF patterns can be implemented in a less pure way. They are just patterns after all, and you can just use them as inspiration for solving your particular problem in the most expedient way. But I think it's closer to the intended application of the pattern if you only construct the object when calling build().
• STerliakov: It doesn't need to be Clone or Copy to return an instance, code in my answer also compiles:) But I agree that patterns should not be applied blindly, they are rather a generic model to consider when building your own solution - sometimes they can be completely usable as-is, but often not. Here storing an instance vs duplicating fields is IMO mostly about ergonomics - that would be important in languages where constructors can have side effects, but it's not the case with struct literals in Rust.

Answer 3

The only arguable objection I have is the missing possibility to detect configuration errors from the calling code.

If it is intended to detect errors only by reading the output and implicitly fall back to a default or last sucessfully set value, then this approach is fine.
If there is a need to react to configuration errors individually (e.g. print "No, you won't get a car from me") then I see two possible solutions:

1. Use configuration functions that report whether the call succeeded or not, e.g. Option<Vehicle> or Result. Unfortunately that would make the configuration less convenient than the functions which return self as you loose the possibility to directly chain the commands.
2. Set a testable error flag or Result in the builder, optionally have one error flag or Result for each build step to be able to locate or resolve the error.

• Davislor: 3. Return an Option<Vehicle> or a Result that can report the type of error?
• Thibe: @Davislor Thanks, I integrated your proposals into my answer
• Davislor: I’d add that you can still chain a function that returns an Option or Result type, with railway-oriented code. For example, VehicleBuilder::new().and_then(foo).

Answer 4

This match statement has a bug in it, as pointed out by Thibe:

self.vehicle.wheels = match wheels {
 1 => {
 eprintln!("Invalid wheel configuration: {}", wheels);
 return self;
 }
 2 => WheelConfiguration::TwoWheeler,
 3 => WheelConfiguration::ThreeWheeler,
 4 => WheelConfiguration::FourWheeler,
 _ => WheelConfiguration::FourPlusWheeler,
};

When wheels is 0, this returns the WheelConfiguration::FourPlusWheeler configuration. The intent of the 4+ wheel condition can be met by using a range of 4.. for the match, instead of a wildcard:

self.vehicle.wheels = match wheels {
 1 => {
 eprintln!("Invalid wheel configuration: {}", wheels);
 return self;
 }
 2 => WheelConfiguration::TwoWheeler,
 3 => WheelConfiguration::ThreeWheeler,
 4 => WheelConfiguration::FourWheeler,
 4.. => WheelConfiguration::FourPlusWheeler,
};

The compiler then spots the bug for you - it gives an error saying that 0 isn't accounted for by the match.

The next problem is that the ranges overlap - FourWheeler is just a special case of FourPlusWheeler. This problem is not detected by the compiler, but it is detected by Clippy. Since FourWheeler is probably intended to be a separate special case, FourPlusWheeler should be renamed to something like FivePlusWheeler or MoreThanFourWheeler that better communicates the intent of it.