4

How do you properly use a builder pattern that expects method chaining in a loop? Using an example from log4rs. Notice self isn't a reference in appender.

//builder pattern from log4rs

pub struct ConfigBuilder {
    appenders: Vec<Appender>,
    loggers: Vec<Logger>,
}

impl ConfigBuilder {
    pub fn appender(mut self, appender: Appender) -> ConfigBuilder {
        self.appenders.push(appender);
        self
    }
}

Doing this below results in an error because (I think) cb is getting moved to the memory being returned by .appender().

let cb = ConfigBuilder::new();
for x in ys {
    cb.appender(x);
}

This below appears to work. Is this the only way to do it?

let mut cb = ConfigBuilder::new();
for x in ys {
    cb = cb.appender(x);
}
marathon
  • 7,881
  • 17
  • 74
  • 137
  • You've made `appender` take ownership and return the new builder, so you need to put the return value somewhere. Did you choose this design on purpose? If you want the first example loop to work instead, you'd need to pass it a mutable self reference instead. – loganfsmyth Aug 05 '17 at 01:11
  • @loganfsmyth - sorry if I wasn't clear, that example is from the log4rs package. https://crates.io/crates/log4rs. Not my design. – marathon Aug 05 '17 at 01:41
  • See also https://stackoverflow.com/q/34362094/155423 – Shepmaster Aug 05 '17 at 03:26
  • Could you post the exact and full error message? It would help a lot. – Lukas Kalbertodt Aug 05 '17 at 08:59
  • Possible duplicate of [Passing a variable to a function (which alters said variable) repeatedly](https://stackoverflow.com/questions/44904677/passing-a-variable-to-a-function-which-alters-said-variable-repeatedly) – Peter Hall Aug 05 '17 at 20:27

1 Answers1

9

Is this the only way to do it?

Semantically it is the critical way, though there are other ways to write it. The appender function takes mut self so it will take ownership of the cb variable's value and make the variable unusable after that point. It could have been designed to borrow a reference, but chaining is nice. Since you're in a loop, the builder needs to be available on the next iteration, so you need to assign the value to something new. This means that

let mut cb = ConfigBuilder::new();
for x in ys {
    cb = cb.appender(x);
}

is indeed one way to do that. Another way would be to use Iterator's .fold to do

let cb = ys.into_iter()
  .fold(ConfigBuilder::new(), |cb, x| cb.appender(x));

which keeps everything in one assignment, but is otherwise pretty much the same.

loganfsmyth
  • 156,129
  • 30
  • 331
  • 251
  • Does the taking ownership and returning ownership cause a lot of unnecessary moves, or is the compiler smart enough to leave the object where it is in this case? – kaya3 Nov 03 '22 at 20:06