5

I was playing around with updating a Rust struct in place using chained methods. I found a way to do this, but I was not sure if my code below was idiomatic Rust versus just a workaround.

In particular, I used .to_owned() at the end of the chained method to return the borrowed struct. The code compiles and works just fine. Here is the minimal example.

//struct.rs
#[derive(Debug, Default, Clone, PartialEq)]
pub struct ModelDataCapture {
    run: i32,
    year: i32,
}
impl ModelDataCapture {
    pub fn new() -> Self {
        ModelDataCapture::default()
    }
    pub fn set_run(&mut self, run: i32) -> &mut ModelDataCapture {
        self.run = run;
        self
    }
    pub fn set_year(&mut self, year: i32) -> &mut ModelDataCapture {
        self.year = year;
        self
    }
}

//main.rs
let data_capture = ModelDataCapture::new()
    .set_run(0)
    .set_year(1)
    .to_owned(); // <<< QUESTION

println!("here is the data capture {:?}", data_capture);

Is this the proper way to write this in-place modification of the struct? If I do not include the .to_owned() method at the end of the chain, the compile fails with a message that the temporary variable does not live long enough.

Shepmaster
  • 388,571
  • 95
  • 1,107
  • 1,366
krishnab
  • 9,270
  • 12
  • 66
  • 123
  • 1
    You might be interested in this crate https://github.com/colin-kiegel/rust-derive-builder#how-it-works :) – kennytm May 16 '17 at 03:27
  • @kennytm thanks for the tip. Yeah, I saw that crate and was just trying to practice writing my own builders before I started to use shortcuts :). But I will look into that the derive-builder crate a bit more. I just want to make sure I can include the input validation logic that I want to include. – krishnab May 16 '17 at 03:37

2 Answers2

7

Your code "works" but doesn't make sense to me. It:

  • Creates a value
  • Mutates the value
  • Clones the value
  • Throws away the original value

See the inefficiency? In addition, all the "in-place mutation" is completely discarded, so there's no benefit to it.

I'd generally introduce a binding to mutate:

let mut data_capture = ModelDataCapture::new();
data_capture.set_run(0).set_year(1);

Or go all the way and create a builder that has some equivalent of finish or build

#[derive(Debug)]
struct ModelDataCapture {
    run: i32,
    year: i32,
}

#[derive(Debug, Default)]
struct ModelDataCaptureBuilder {
    run: i32,
    year: i32,
}

impl ModelDataCaptureBuilder {
    fn set_run(self, run: i32) -> Self {
        ModelDataCaptureBuilder { run, ..self }
    }

    fn set_year(self, year: i32) -> Self {
        ModelDataCaptureBuilder { year, ..self }
    }

    fn build(self) -> ModelDataCapture {
        let ModelDataCaptureBuilder { run, year } = self;
        ModelDataCapture { run, year }
    }
}

fn main() {
    let data_capture = ModelDataCaptureBuilder::default().set_run(0).set_year(1).build();

    println!("here is the data capture {:?}", data_capture);
}

See Do Rust builder patterns have to use redundant struct code? for more examples of builders that mirror the built items.

You could take self by-value in the first example, but that's annoying in most cases, as you always have to remember to bind the result.

Community
  • 1
  • 1
Shepmaster
  • 388,571
  • 95
  • 1,107
  • 1,366
  • Okay great. Yeah, I see what you mean by the create, mutate, clone idea when I chain methods before binding. That is why I was wondering about the strange `.to_owned()` method. Thanks for the tip. I am starting to get the hang of this better than before. – krishnab May 16 '17 at 03:29
  • Wouldn't it be better to make direct use of the move semantics in setters that consume the object anyway? I.e. define `set_run` as `fn set_run(mut self, run: i32) -> Self { self.run = run; self }`, and so on. Although the compiler might be smart enough to optimize both to the exact same output, being explicit about updating a single field feels more efficient and somewhat clearer. – user4815162342 May 16 '17 at 09:00
-1

You could change the function to take ownership over self and return self. Because each "setter" method returns the ownership of self, this code should work out nicely. For more information, please checkout the rust book

//struct.rs
#[derive(Debug, Default, Clone, PartialEq)]
pub struct ModelDataCapture {
    run: i32,
    year: i32,
}
impl ModelDataCapture {
    pub fn new() -> Self {
        ModelDataCapture::default()
    }
    pub fn set_run(mut self, run: i32) -> ModelDataCapture {
        self.run = run;
        self
    }
    pub fn set_year(mut self, year: i32) -> ModelDataCapture {
        self.year = year;
        self
    }
}

fn main() {
    //main.rs
    let data_capture = ModelDataCapture::new().set_run(0).set_year(1);

    println!("here is the data capture {:?}", data_capture);
}
whati001
  • 317
  • 2
  • 9