CODEWITHSUNDEEP
rust
Mohammad gorgan mohammadi
Mohammad gorgan mohammadi

Reputation: 415

why do we need to call take() for Option<T> variable

In this piece of code:

pub struct Post {
    state: Option<Box<dyn State>>,
    content: String,
}

impl Post {
    pub fn new() -> Post {
        Post {
            state: Some(Box::new(Draft {})),
            content: String::new(),
        }
    }
    
    pub fn add_text(&mut self, text: &str) {
        self.content.push_str(text);
    }
    
    pub fn content(&self) -> &str {
        ""
    }

    pub fn request_review(&mut self) {
        if let Some(s) = self.state.take() {
            self.state = Some(s.request_review())
        }
    }
}

trait State {
    fn request_review(self: Box<Self>) -> Box<dyn State>; 
}

struct Draft {}

impl State for Draft {
    fn request_review(self: Box<Self>) -> Box<dyn State> {
        Box::new(PendingReview {})
    }
}

struct PendingReview {
    fn request_review(self: Box<Self>) -> Box<dyn State> {
        self
    }
}

there is a call to take(); the book says:

To consume the old state, the request_review method needs to take ownership of the state value. This is where the Option in the state field of Post comes in: we call the take method to take the Some value out of the state field and leave a None in its place.

We need to set state to None temporarily rather than setting it directly with code like self.state = self.state.request_review(); to get ownership of the state value. This ensures Post can’t use the old state value after we’ve transformed it into a new state.

How is it possible that Post uses its old state if we set it directly?

Upvotes: 31

Views: 13475

Answers (5)

Kodra
Kodra

Reputation: 169

It's my personal opinion, but I think the example in Rust Book is nonsense.

If it were me implementing the same logic, it would be like:

pub struct Post {
    state: Box<dyn State>,
    content: String,
}

impl Post {
    pub fn new() -> Post {
        Post {
            state: Box::new(Draft {}),
            content: String::new(),
        }
    }

    pub fn request_review(&mut self) {
        self.state = self.state.request_review();
    }
}

trait State {
    fn request_review(&self) -> Box<dyn State>; 
}

struct Draft {}

impl State for Draft {
    fn request_review(&self) -> Box<dyn State> {
        Box::new(PendingReview {})
    }
}

struct PendingReview {}

impl State for PendingReview {
    fn request_review(&self) -> Box<dyn State> {
        Box::new(PendingReview {})
    }
}

Without Option and take() at all.

The logic behind the implementation in The Book:

  1. The reason why state needs to be an Option is that request_review() consumes the ownership.
  2. The reason why request_review() consumes ownership is that PendingState returns self.
  3. The reason why PendingState#request_review() return self is... to show that you can do it, as a teaching book, I guess. Or as a micro-optimization, just in case at a later point PendingState becomes a huge struct and it's expensive to allocate one.

If it just returns a new Box::new(PendingReview{}) you don't need any of these complexity.

Upvotes: 1

tlid
tlid

Reputation: 166

I don't think the fact that the data is on the heap, as mentioned by @xiang-zhou, is the fundamental issue here. The following doesn't compile because the call to Blog::request_review tries to move the state field out of self:

impl Blog {
    pub fn request_review(&mut self) {
        self.state = self.state.request_review()
    }
}

impl State for Draft {
    fn request_review(self: Box<Self>) -> Box<dyn State> {
        Box::new(PendingReview {})
    }
}

However, this isn't permitted by the compiler because self and self.state are behind a reference. This would be a problem for data on the stack as well:

error[E0507]: cannot move out of `self.state` which is behind a mutable reference
  --> src/lib.rs:23:22
   |
23 |         self.state = self.state.request_review()
   |                      ^^^^^^^^^^ ---------------- `self.state` moved due to this method call
   |                      |
   |                      move occurs because `self.state` has type `Box<dyn State>`, which does not implement the `Copy` trait

To get around this, Option::take is used because it allows moving a value out from a &mut reference, something that is not usually allowed. The source code for Option::take and mem::replace rely on carefully implemented unsafe rust to allow this.

Upvotes: 11

User 10482
User 10482

Reputation: 1012

Quote from the book; bold font my formatting

we call the take method to take the Some value out of the state field and leave a None in its place, because Rust doesn’t let us have unpopulated fields in structs.

As for move and assignment in the same statement, looks like Rust does not detect it as valid.

Upvotes: 1

Xiang Zhou
Xiang Zhou

Reputation: 172

If you code like this:

pub struct Post {
    state: Box<dyn State>,
    content: String,
}

trait State {
    fn request_review(self: Box<Self>) -> Box<dyn State>; 
}

impl Post {
    // ... 
    pub fn request_review(&mut self) {
        self.state = self.state.request_review();
    }
    // ... 
}

You will get a compiler error:

self.state = self.state.request_review();
             ^^^^^^ move occurs because `self.state` has type `std::boxed::Box<dyn State>`, which does not implement the `Copy` trait'.

This is because calling State::request_review will move Box<self>, which is allocated on heap, and Rust doesn't allow you to just move values away from heap unless you implement Copy, otherwise what's left there? The book uses Option::take() to move ownership out and leave None on the place.

Upvotes: 14

Gr&#233;gory OBANOS
Gr&#233;gory OBANOS

Reputation: 1051

If request_review panic, it will lead to free the Box twice, first in request_review and then when the Option is freed.

Upvotes: -1

Related Questions