Asking for help with Chapter 17.3 in the book

Please see the code in the playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=2cbfc0ce9c9daf317b13c47b1557a739

I am trying to implement the suggestions from the chapter:

" The implementation using the state pattern is easy to extend to add more functionality. To see the simplicity of maintaining code that uses the state pattern, try a few of these suggestions:

  • 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 ."

My implementation for the third feature involves having the state objects implement an add_text function that takes the text to be added as a parameter. Only the draft objects will return the text, the rest will substitute it with an empty string. Therefore the Post objects will then push whatever is returned.

I get the following compiler error on line 17:

self.content.push_str(self.state.as_ref().unwrap().add_text(text));
   |                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ move occurs because value has type `std::boxed::Box<dyn State>`, which does not implement the `Copy` trait
  1. Am I going about it the correct way? 2) How would I go about fixing this error? I understand it has something to do with ownership.

Please note I am a complete beginner.

your State trait functions need some fixes.

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=2ff737bbfe39307bf5e4131c08577491

I modified:

  • Post::add_text
  • State::add_text

Thanks, this compiles but how does it satisfy the requirement " Allow users to add text content only when a post is in the Draft state."?

If I understand correctly, now the following code inside Post::add_text will always be called regardless of the state of the post:

self.content.push_str(text);

Mh. Maybe enums would be more suitable as then you can just check what state it is.

Another option would be to return a bool to indicate whether the text was added.
Ultimately you may still end up with some text cleanup meaning you have to fetch the new text anyway.

After changing the definition of the add_new function it now compiles but would still appreciate comments if this is the 'correct' solution as per the book: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=1b5c3abbd34a207f0dc1ba68b9aad69d

Oddly enough I was reading the same chapter and section today. :smiley:

I actually interpret this as you should only add_text on Draft state. If you are other state you should not replace the existing text, nor append to it, but keep the existing one.
The original code should append (push_str), your code is deleting it.

I think the whole point of this third task is to make also add_text follow the state pattern (with delegation). Earlier in the chapter they say.
" This behavior doesn’t depend on the state the post is in, so it’s not part of the state pattern. The add_text method doesn’t interact with the state field at all, but it is part of the behavior we want to support."
So basically, here it is time to undo it.

To be honest I do not like much the state pattern implementation described on the book. I dont like that you need the Option and I dont like that there are so many overriden methods just to return self. An important point in the patter in other languages is that quite often states implementation become trivial (1 or 2 methods) because there is a default up.
I am working on my own try. If I come up with something, I will also share with you.

Thanks a lot for the explanation, there are so many concepts in the book and it's really helpful to get input from others. Just some clarification about my implementation, I don't think it replaces the text. The text is appended by the add_text method in the Post object here:

pub fn add_text(mut self, text: &str) {
        self.content.push_str(self.state.as_ref().unwrap().add_text(text));
    }

The add_text method of the Post object takes the text to be appended as a parameter. But rather than appending this text directly, it passes it as a parameter to the add_text method of the state object which returns a string. Only if the state object is a Draft will it return the text that was to be appended. In other cases it will return an empty string, meaning Post will append an empty string i.e. it will not append anything.

I was trying to satisfy the requirement in the hint:

" Hint: have the state object responsible for what might change about the content but not responsible for modifying the Post"

Ok. After a lot of sweat I have something that is minimally satisfactory State Pattern implementation in my opinion.

As I mentioned before, I did not like that all the time one needed to work on the option just to delegate. But that was not the worse part. I think it is unacceptable to all the time repeat all the methods on every State. The State Pattern is about making new implementations centralized and flexible. Adding a lot of methods everywhere beats the purpose for me.

So here is my take (still not did the exercise - just trying to improve over the original code of the book).
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=a0d0ce2bf28486f24b99f00417edbab8

It took me a long time. I tried in different ways and finally understood the term "fighting the borrow checker". :sweat_smile: I also tried with references and freaking lifetimes everywhere... Got upset and thought about buying a Go language book instead. :stuck_out_tongue: But it is kind of ok in the end.
I realized all the borrow check errors I saw, and the things I disliked on the book implementation are to work around that in Rust and object cannot delegate to another object to change itself.
So, I centralized the state change on the Post class itself in a new method check_state_change. To accomplish that I needed to add an extra type, but the main trick there is that enum type has a KeepState which can be the default implementation on the trait. So instead of fighting the Option, I embraced the technique but applied to the problem at hand.
I also got rid of all Box references on the states. The box is only where dynamic dispatching is needed.

Well, now I will make the improvements suggested as exercise.

Ah ok. I tried to run your last playground, but did not try too hard. So maybe I did not understand everything.

Now that´s what I am talking about!

After my initial improvements, it became really easy to do further changes!
The State Pattern and many other design patterns are about making further changes easy.
I have also modified the main to embed the testing of the new requirements
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=e91b63418b158305ba9f8232d239f658