book icon indicating copy to clipboard operation
book copied to clipboard

Execercise in chapter 17.3 can be confusing

Open videbar opened this issue 3 years ago • 5 comments

  • [x] I have checked the latest main branch to see if this has already been fixed
  • [x] I have searched existing issues and pull requests for duplicates

URL to the section(s) of the book with this problem:

https://doc.rust-lang.org/book/ch17-03-oo-design-patterns.html#implementing-transitions-as-transformations-into-different-types

Description of the problem:

During chapter 17.3 it is explained how the same system could be implemented first using the state design pattern and then a variation of this pattern that takes advantage of some of rust features. The system being explained is a blogpost that can be in different states: draft, pending review and post.

It is explained how to implement the blogpost using the state design pattern and a simple exercise is left for the reader:

  • Add a reject method that changes the post’s state from PendingReview back to Draft.
  • Require two calls to approve before the state can be changed to Published.
  • Allow users to add text content only when a post is in the Draft state. Hint: have the state object responsible for what might change about the content but not responsible for modifying the Post.

Which is great.

Then it is explained how this pattern can be modified to use rust features and at the end of the chapter it says:

Try the tasks suggested for additional requirements that we mentioned at the start of this section on the blog crate as it is after Listing 17-20 to see what you think about the design of this version of the code.

I think this is a problem because, at least to the best of my knowledge, there's no easy way no implement the second requirement in the new design:

Require two calls to approve before the state can be changed to Published.

I found this post that explains the problems with this exercise and the best solution states:

I think the idea that they are getting at is that you add another type, so that you have PendingTwoReviewsPost and PendingOneReviewPost or the like, and approving the former returns the latter, and approving the latter returns a Post. I.e., encoding the state into the types.

which seems to be the only way to solve the problem without modifying how the API is called (which I think is the intention). If this is indeed the intended solution, it's awful, if I wanted to change the number of required approvals from 2 to let's say 5, instead of changing a parameter in the script, I would need to define 3 new structs!

Suggested fix:

Replace the line:

Try the tasks suggested for additional requirements that we mentioned at the start of this section on the blog crate as it is after Listing 17-20 to see what you think about the design of this version of the code.

with something like:

This design has other problems however, if you try the tasks suggested for additional requirements, you will realize that there is no easy way to change the number of approvals required for a post to change from PendingReview to Post.

videbar avatar Nov 12 '21 16:11 videbar

I think "not useful" is a bit harsh. I think this patterns merits a discussion about the tradeoffs of the OO state pattern versus this pattern of consuming the current struct to produce a new struct (is there a good name for this pattern in Rust?!!??)

I am also currently learning Rust and going through ch17 and I'd like to say that the pattern that is being described at the end of ch17 is absolutely brilliant in certain circumstances! For example, imagine you can allocate memory that can only be in one of three states: read-only, read/write, and read/execute (so that you can run code from that memory 😉). As the names imply, you can read in all states, but you can only write in the read/write state, and you can only run code in the read/execute state.

A state-machine showing allocations, and their three different states, and the transitions between them

The pattern in the Rust book at the end of ch17 makes it impossible to do something silly like, attempt to write to a read-only area; however, Rust will still allow you to move into the desired state by changing types (note: .into_*() methods consume the input, returning a different type). This is enforced by the type system at compile-time. For example, I want to write a program that writes some code into the allocation, but I need to convert it to read-only, and then convert it into an executable and run the program I wrote, Rust allows me to write this:

let alloc = Allocation::new();

// allocation is initially read-only; let's make it writable so we can write a program into it!
let alloc = alloc.into_writable();
alloc.write_data("xor eax, eax");

// my system won't allow me write AND execute at the same time, so I need to go back to read-only...
let alloc = alloc.into_read_only();

// Okay, we can execute it now
let alloc = alloc.into_executable();
alloc.run_program();

Using the type system this way, writing to read-only memory is impossible and executing writable memory is also impossible. (You can even do nifty things like implement mut traits only on the read-write state type!) Using the OO state pattern, you could make it so that, when you're not in the ReadAndWrite state, .write_data() ignores the read, or perhaps returns some kind of Err(...), or it could even panic. However using types in this way makes these undesirable scenarios impossible, which I think is really darn neat!

eddieantonio avatar Nov 22 '21 22:11 eddieantonio

I think "not useful" is a bit harsh. I think this patterns merits a discussion about the tradeoffs of the OO state pattern versus this pattern of consuming the current struct to produce a new struct

Agreed, I just edited the title.

I am also currently learning Rust and going through ch17 and I'd like to say that the pattern that is being described at the end of ch17 is absolutely brilliant in certain circumstances!

I don't intend to say that it can't be useful in some scenarios, like the one you're describing. My problem is that it doesn't work if you want to implement a system such that, when you call a certain method, may or may not change states based on some condition.

In your example, you may want that, when you call into_writable(), the memory object only changes from ReadOnly to ReadAndWrite if no other application is reading it. Now you need the method into_writable() to return something which can either be ReadOnly or ReadAndWrite and there's no easy way of writing the signature of such function.

In the book, that's exactly what the exercise is asking you to do by telling you to require two calls to the method approve() before moving from PendingReviewPost to Published. You would like approve() to return either a PendingReviewPost or a Published depending on the number of approvals, but that's not possible.

videbar avatar Nov 23 '21 14:11 videbar