The best way to return a value from a builder

help
1

Here is some example code that has the APIs I want, one builder-style, the other multi-line (but it doesn't work due to one line):

fn main() {
    let t1 = ThingBuilder::new().name("one").pos(5)
        .add("Alpha").add("Bravo").build();
    dbg!(&t1);
    let mut tb2 = ThingBuilder::new();
    tb2.add("Charlie").add("Delta").add("Echo");
    tb2.name("two");
    tb2.pos(-4);
    let t2 = tb2.build();
    dbg!(&t2);
}

#[derive(Debug)]
struct Thing {
    name: String,
    pos: i32,
    data: Vec<String>,
}

impl Thing {
    fn new() -> Thing {
        Thing{name: "".to_string(), pos: 0, data: vec![]}
    }
}

#[derive(Debug)]
struct ThingBuilder {
    thing: Thing,
}

impl ThingBuilder {
    fn new() -> ThingBuilder {
        ThingBuilder{thing: Thing::new()}
    }

    fn name<'a>(&'a mut self, name: &str) -> &'a mut Self {
        self.thing.name = name.to_string();
        self
    }

    fn pos<'a>(&'a mut self, pos: i32) -> &'a mut Self {
        self.thing.pos = pos;
        self
    }

    fn add<'a>(&'a mut self, datum: &str) -> &'a mut Self {
        self.thing.data.push(datum.to_string());
        self
    }

    fn build(&self) -> Thing {
        self.thing
    }
}

The error is in the build function:

error[E0507]: cannot move out of borrowed content
  --> src/main.rs:52:9
   |
52 |         self.thing
   |         ^^^^^^^^^^ cannot move out of borrowed content

error: aborting due to previous error

I understand what it is saying but don't know the best solution. What I'd like to do is move self.thing into a temporary thing, then replace self.thing with a Thing::new(), and then return the temporary thing. Unfortunately that doesn't work. Would making self.thing an Option allow the build() method return a plain Thing? I'm trying to move the Thing once it is built rather than copy it because the real thing that this code is serving as a model for will be much heavier.
Can anyone advise?

2

You'll need build(&mut self), since you're modifying it, then std::mem::replace(&mut self.thing, Thing::new()) returns the old value.

1 Like
3

Thank you, that works perfectly!

For other beginners, here's the corrected method:

    fn build(&mut self) -> Thing {
        std::mem::replace(&mut self.thing, Thing::new())
    }
2 Likes
4

One other question on this: If Thing's fields are private and Thing is in a different module from ThingBuilder am I right in assuming that my choices are to make the fields public or provide public setters because there's no equivalent of C++ "friend" classes in rust?

5

I would suggest making build taking self as value, not as mutable reference so it will be consumed (which will makes sense, because its the builder (?)).

6

(The build() method can't take plain self because returning self.thing then produces a cannot move out of borrowed content error.)

Here's a greatly cut down example:

struct PlanBuilder {
    plan: Plan,
}

impl PlanBuilder {
    fn new() -> PlanBuilder {
        PlanBuilder{plan: Plan::default()}
    }

    fn path<'a>(&'a mut self, path: &str) -> &'a mut Self {
        self.plan.path = path.to_string();
        self
    }

    fn build(&mut self) -> Plan {
        std::mem::replace(&mut self.plan, Plan::default())
    }
}

This is in a separate module from Plan and to make it work I have to make Plan.path public, which I'd prefer not to do.

7

But what's the reasoning behind this, if we are allowed to know?

8

Accident more than design! But the question you raise makes me think that I should put the builder inside the Plan module & then try and keep the fields private.

[edit] I've now been able to make the Plan fields private by making the builder part of the Plan module.

Thanks.

9 
fn build(self) -> Plan {
    self.plan
}

works

10

It doesn't for me:

fn main() {
    let t1 = ThingBuilder::new().name("one").pos(5)
        .add("Alpha").add("Bravo").build();
    dbg!(&t1);
    let mut tb2 = ThingBuilder::new();
    tb2.add("Charlie").add("Delta").add("Echo");
    tb2.name("two");
    tb2.pos(-4);
    let t2 = tb2.build();
    dbg!(&t2);
}

#[derive(Debug)]
struct Thing {
    name: String,
    pos: i32,
    data: Vec<String>,
}

impl Thing {
    fn new() -> Thing {
        Thing{name: "".to_string(), pos: 0, data: vec![]}
    }
}

#[derive(Debug)]
struct ThingBuilder {
    thing: Thing,
}

impl ThingBuilder {
    fn new() -> ThingBuilder {
        ThingBuilder{thing: Thing::new()}
    }

    fn name<'a>(&'a mut self, name: &str) -> &'a mut Self {
        self.thing.name = name.to_string();
        self
    }

    fn pos<'a>(&'a mut self, pos: i32) -> &'a mut Self {
        self.thing.pos = pos;
        self
    }

    fn add<'a>(&'a mut self, datum: &str) -> &'a mut Self {
        self.thing.data.push(datum.to_string());
        self
    }

    fn build(self) -> Thing {
        self.thing
    }
}

Here's the error I get:

error[E0507]: cannot move out of borrowed content
 --> src/main.rs:2:14
  |
2 |       let t1 = ThingBuilder::new().name("one").pos(5)
  |  ______________^
3 | |         .add("Alpha").add("Bravo").build();
  | |__________________________________^ cannot move out of borrowed content

Fortunately the std::mem::replace() solution works perfectly so I'm using that.

11

It is because you are borrowing your builder all the time. I suggest do it this way:

struct PlanBuilder {
    plan: Plan,
}

impl PlanBuilder {
    fn new() -> PlanBuilder {
        PlanBuilder{plan: Plan::default()}
    }

    fn path(mut self, path: &str) -> Self {
        self.plan.path = path.to_string();
        self
    }

    fn build(self) -> Plan {
        self.plan
    }
}

This is exaclty how I make my builders. There is also possibility to take self instead of mut self for intermediate methods, and reconstructing objects like this:

fn path(self, path: &str) -> Self {
    Self {
        plan: {
            path: path.to_string(),
            ..self.plan
        },
        ..self
    }
    self.plan.path = path.to_string();
    self
}

But I personally don't like this attitude. In general I strongly advice to avoid returning &'a mut Self - it allows chaining, but the problem is, that chained values are borrows, and you cannot do something like:

let builder = PlanBuilder::new().path("path");
do_additional_buildiing(builder);

Instead of this, you need to do additional boiler plate:

let builder = PlanBuilder::new();
builder.path("path");
do_additional_building(builder);
1 Like
12

Exactly my thoughts but even the stdlib does the &mut self stuff OpenOptions in std::fs - Rust

13

Stdlib does many strange things in many places. I guess that some of them, maybe all of them are pretty well justified, but it doesn't mean, that is always the best way. This exact case is very specific, because building doesn't need to consume builder, and also all building options are (almost) always known in place where file is created, which is not typical builder use case.

14

You can use pub(crate), pub(super), or pub(in some::path) to restrict visibility.

15

The way you describe works and is fine for the builder pattern. But if you create a builder and then separately (e.g., in if statements) want to change it then things get less appealing.

let t1 = Builder::new().one(1).two(2).build(); // fine
let b = Builder::new();
b = b.one(1);
b = if extra { b.two(2) } else { b };
let t2 = b.build();

Whereas right now using the &mut Self stuff I can do this:

let t1 = Builder::new().one(1).two(2).build(); // fine
let mut b = Builder::new();
b.one(1);
if extra { b.two(2) }
let t2 = b.build();

which I find more appealing.

16

It is actually true, I didn't point it out because I don't fall in problem very often, but yes, this may be considered as disadvantage of this attitude.