book

book copied to clipboard

• I have searched open and closed issues and pull requests for duplicates, using these search terms:

  • self.state.request_review

• I have checked the latest main branch to see if this has already been fixed, in this file:

  • https://github.com/rust-lang/book/blob/main/src/ch17-03-oo-design-patterns.md

URL to the section(s) of the book with this problem: https://doc.rust-lang.org/book/ch17-03-oo-design-patterns.html#requesting-a-review-of-the-post-changes-its-state

Description of the problem: The following sentence is still very confusing:

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.

First, "We need" implies that self.state = self.state.request_review() gives the incorrect result. However, it actually gives the correct result; from https://github.com/rust-lang/book/issues/837, the intent here is to make the code less error-prone in case that line is later commented out.

Additionally. "This ensures Post can’t use the old state value after we’ve transformed it into a new state." seems misleading too. Post already cannot use the old state value, since the self parameter is moved in request_review(self: Box<Self>). This would only "ensure" if we for some reason changed the self parameter to borrow.

Suggested fix:

Please add a paragraph making it clear that the usage of Option is solely to make the code less error-prone in the face of future refactorings, and not explicitly needed for correctness.