Solving an an exercise from the Chapter 17 of TRPL

[lessless]

Hi folks,

I have a question about an exercise from the "Implementing an Object-Oriented Design Pattern" in the TRPL book. In chapter 17, after the Blog app is refactored to type-based state transitions, the reader is asked to implement an additional requirement:

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

The way I understand the programming model, it means that the approve function of the PendingReviewPost should return either PendingReviewPost or Post, depending on a number of approvals:

pub struct PendingReviewPost {
    content: String,
    num_of_approvals: usize,
}

impl PendingReviewPost {
    pub fn approve(self) -> Post {
        if self.num_of_approvals == 1 {
            Post {
                content: self.content,
            }
        } else {
            self.num_of_approvals += 1;
            self
        }
    }
}

But function can have only one return type.

Option 1

As it was pointed out in Discord, it would be possible to solve this by adding more interim types and more functions:

impl PendingReviewPost {
  pub fn add_first_approval() -> PostWithOneApproval
  pub fn add_second_approval() -> ApprovedPost
}

but it's not something that I would typically do because:

1. That would expose client code to unnecessary implementation details and eventually will lead to a coupling between supposedly independent parts of the codebase
2. PendingPost whether with zero approvals or one approval is still a PendingPost. Next software developer to read that code would scratch his head trying to understand the rationale behind that design.
   2.1 There would be a necessity for a mechanism to share common behaviour between those two types.

Option 2

Another approach would be to introduce an enum to capture either one of those outcomes:

enum EitherPreApprovedOrApproved {
    PreApproved(PendingReviewPost),
    Approved(Post),
}

pub fn approve(self) -> EitherPreApprovedOrApproved {
    if self.num_of_approvals == 1 {
        EitherPreApprovedOrApproved::Approved(Post {
            content: self.content,
        })
    } else {
        self.num_of_approvals += 1;
        EitherPreApprovedOrApproved::PreApproved(self)
    }
}

But again, this code would put a client in the unfavourable position of extracting value from approve() and basically putting onto it a burden of dealing with state transitions.

So I believe there should be other solutions to this problem. Would anyone please enlighten me with an idiomatic solution to this problem?

Thank you and Happy New Year !

[jhwgh1968]

I have not dug into the example, and this might be overkill. But my first instinct would be to use type states.

Something like this (off the top of my head):

trait ApproveLevel {}

struct NoApprovals();
impl ApproveLevel for NoApprovals {}

struct OneApproval(String);
impl ApproveLevel for OneApproval {}

struct TwoApprovals(String, String);
impl ApproveLevel for TwoApprovals {}

struct Post<L: ApproveLevel> {
    approvals: L,
    // includes contents, etc.
}

impl Post<NoApprovals> {
    pub fn approve(self, who: &str) -> Post<OneApproval> {
        Post {
            approvals: OneApproval(who.to_string()),
            ..self
        }
    }
}

impl Post<OneApproval> {
    pub fn approve(self, who: &str) -> Result<Post<TwoApprovals>, Post<OneApproval>> {
        if who == &self.approvals.0 {
            Err(self)
        } else {
            Ok(Post {
                approvals: TwoApprovals(self.approvals.0.clone(), who.to_string()),
                ..self
            })
        }   
    }
}

fn post_item(post: Post<TwoApprovals>) { /* ... */ }
fn main() {
    println!("Hello, world!");
}

EDIT: cleanup and playground link.

[lessless]

whoa, that's quite an interesting pattern! thanks for sharing.

the only weak point, IMO, in this approach is that all state transitions ( except for the approve return a specific type of post, whilst approve returns Result.

pub struct Post {
    content: String,
}

pub struct DraftPost {
    content: String,
}

pub struct PendingReviewPost {
    content: String,
}

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

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

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

    pub fn request_review(self) -> PendingReviewPost {
        PendingReviewPost {
            content: self.content,
        }
    }
}

impl PendingReviewPost {
    pub fn approve(self) -> Post {
        Post {
            content: self.content,
        }
    }

    pub fn reject(self) -> DraftPost {
        DraftPost {
            content: self.content,
        }
    }
}


fn main() {}

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

    #[test]
    fn approved_postent_content_is_displayed() {
        let mut post = Post::new();
        post.add_text("I ate a salad for lunch today");
        let post = post.request_review();
        let post = post.approve();
        assert_eq!("I ate a salad for lunch today", post.content());
    }

    #[test]
    fn pending_post_can_be_rejected() {
        let mut post = Post::new();
        post.add_text("I ate a salad for lunch today");
        let post = post.request_review();
        assert!(matches!(post.reject(), DraftPost { .. }));
    }
}

Rust Playground

That puts the burden of dealing with an "exceptional" case on a client code.
The requirement of "two approvals" sounds like something that is going to change and at that moment all the coupling created between client and Post implementation will cause an imminent pain

[jhwgh1968]

The only reason I had it return Result was because it wasn't clear to me the approvers were distinct. That is, it is a runtime check to ensure that it is approved by Alice and Bob, not Alice and Alice.

Since there is only one external "client", I presumed the client should have to handle that. I called it result, but it is not necessarily an "error" if it fails. You simply get the original object back. It is a similar to what happens if you fail to downcast a dyn Any box.