Designing posts: state machine

Question

In the Object-Oriented Design Pattern Implementation chapter of the second edition of The Rust Programming Language, the basic code structure is given and it suggests trying to add features:

This implementation is easy to extend to add more functionality. Here are some changes you can try making to the code in this section to see for yourself what it's like to maintain code using this pattern over time:

• Only allow adding text content when a post is in the Draft state
• Add a reject method that changes the post's state from PendingReview back to Draft
• Require two calls to approve before changing the state to Published

Please critique my approach:

struct Post {
    content: String
}

struct DraftPost {
    content: String
}

struct PendingPost {
    content: String,
    approvals: u32

}

impl Post {
    fn new() -> DraftPost {
        DraftPost { content: String::new() }
    }

    fn content(&self) -> &str {
        &self.content
    }
}

impl DraftPost {
    fn req_review(self) -> PendingPost {
        PendingPost {
            content: self.content,
            approvals: 0
        }
    }

    fn add_text(&mut self, content: &str) {
        self.content.push_str(content);
    }
}

enum PublishResult {
    PendingPost(PendingPost),
    Post(Post)
}

impl PendingPost {
    fn approve(&mut self) {
        self.approvals += 1;
    }
    fn reject(self) -> DraftPost {
        DraftPost { content: self.content }
    }
    fn publish(self) -> PublishResult {
        if self.approvals > 1 {
            PublishResult::Post(Post{content: self.content})
        } else {
            PublishResult::PendingPost(self)
        }
    }
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn publish_workflow() {
        let mut draft = Post::new();
        draft.add_text("ashish first post");
        let mut pending = draft.req_review();
        pending.approve();
        pending.approve();
        let publish = pending.publish();
        match publish {
            PublishResult::Post(p) => assert_eq!(p.content(),
                                                 "ashish first post"),
            _ => assert!(false)
        }
    }

    #[test]
    fn reject_workflow() {
        let mut draft = Post::new();
        draft.add_text("ashish first post");
        let pending = draft.req_review();
        let mut again_draft = pending.reject();
        again_draft.add_text(".. after first one..");
    }

    #[test]
    fn two_approvals_workflow() {
        let mut draft = Post::new();
        draft.add_text("ashish first post");
        let mut pending = draft.req_review();
        pending.approve();
        match pending.publish() {
            PublishResult::PendingPost(_) => assert!(true),
            _ => assert!(false)
        }
    }
}

Answer 1

Overall, everything seems fine. Some minor nits and suggestions:

1. Stylistically, you should have:

   • trailing commas
   • spaces on struct literals (Post { content: self.content }

2. PublishResult makes me think it's a type alias of std::result::Result. I'd advocate for picking a different name.

3. Instead of matching in your tests and having the not-very-useful assert!(true) and assert!(false), add some helper methods to PublishResult to convert to an expected variant. This is a common pattern for this style of enum. Then you can expect with text in your tests

4. Dnt ndlssly abbr mthds (Don't needlessly abbreviate methods). Call it request_review instead of req_review.

5. It's strange to have a test (reject_workflow) with no assertions. I don't think anything's wrong with it, as it's testing the methods and types, it's just a bit implicit.

struct Post {
    content: String,
}

struct DraftPost {
    content: String,
}

struct PendingPost {
    content: String,
    approvals: u32,
}

impl Post {
    fn new() -> DraftPost {
        DraftPost { content: String::new() }
    }

    fn content(&self) -> &str {
        &self.content
    }
}

impl DraftPost {
    fn request_review(self) -> PendingPost {
        PendingPost {
            content: self.content,
            approvals: 0,
        }
    }

    fn add_text(&mut self, content: &str) {
        self.content.push_str(content);
    }
}

enum PublishResult {
    PendingPost(PendingPost),
    Post(Post),
}

impl PublishResult {
    fn into_pending_post(self) -> Option<PendingPost> {
        use PublishResult::*;
        match self {
            PendingPost(v) => Some(v),
            _ => None,
        }
    }

    fn into_post(self) -> Option<Post> {
        use PublishResult::*;
        match self {
            Post(v) => Some(v),
            _ => None,
        }
    }
}

impl PendingPost {
    fn approve(&mut self) {
        self.approvals += 1;
    }
    fn reject(self) -> DraftPost {
        DraftPost { content: self.content }
    }
    fn publish(self) -> PublishResult {
        if self.approvals > 1 {
            PublishResult::Post(Post { content: self.content })
        } else {
            PublishResult::PendingPost(self)
        }
    }
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn publish_workflow() {
        let mut draft = Post::new();
        draft.add_text("ashish first post");
        let mut pending = draft.request_review();
        pending.approve();
        pending.approve();
        let post = pending.publish().into_post().expect("not a post");
        assert_eq!(post.content(), "ashish first post");
    }

    #[test]
    fn reject_workflow() {
        let mut draft = Post::new();
        draft.add_text("ashish first post");
        let pending = draft.request_review();
        let mut again_draft = pending.reject();
        again_draft.add_text(".. after first one..");
    }

    #[test]
    fn two_approvals_workflow() {
        let mut draft = Post::new();
        draft.add_text("ashish first post");
        let mut pending = draft.request_review();
        pending.approve();
        pending
            .publish()
            .into_pending_post()
            .expect("Not a pending post");
    }
}