#New to Rust: what's a good way to refactor this snippet?

15 messages Ā· Page 1 of 1 (latest)

slate shell
#

I'm working through the 100 exercises and this is my solution for setters.

The goal was to add value checks that panic whenever you would attempt to change a field on the ticket.
However, there's repeated logic into it (new has the same checks as set_title, set_description, and set_status have).

I would like to extract them to separate functions, and just do the function calls.

pub struct Ticket {
    title: String,
    description: String,
    status: String,
}

impl Ticket {
    pub fn new(title: String, description: String, status: String) -> Ticket {
        if title.is_empty() {
            panic!("Title cannot be empty");
        }
        if title.len() > 50 {
            panic!("Title cannot be longer than 50 characters");
        }
        if description.is_empty() {
            panic!("Description cannot be empty");
        }
        if description.len() > 500 {
            panic!("Description cannot be longer than 500 characters");
        }
        if status != "To-Do" && status != "In Progress" && status != "Done" {
            panic!("Only `To-Do`, `In Progress`, and `Done` statuses are allowed");
        }

        Ticket {
            title,
            description,
            status,
        }
    }

    pub fn title(&self) -> &String {
        &self.title
    }

    pub fn description(&self) -> &String {
        &self.description
    }

    pub fn status(&self) -> &String {
        &self.status
    }

    pub fn set_title(&mut self, new_title: String) {
        if new_title.is_empty() {
            panic!("Title cannot be empty");
        }
        if new_title.len() > 50 {
            panic!("Title cannot be longer than 50 characters");
        }
        self.title = new_title;
    }

    pub fn set_description(&mut self, new_description: String) {
        if new_description.is_empty() {
            panic!("Description cannot be empty");
        }
        if new_description.len() > 500 {
            panic!("Description cannot be longer than 500 characters");
        }

        self.description = new_description;
    }

    pub fn set_status(&mut self, new_status: String) {
        if new_status != "To-Do" && new_status != "In Progress" && new_status != "Done" {
            panic!("Only `To-Do`, `In Progress`, and `Done` statuses are allowed");
        }

        self.status = new_status;
    }
}

The way I approached this was, thinking that I should call a function like validate_status(new_status) etc., which basically takes just the reference and panics if it's invalid, without returning anything if it doesn't panic (note: panicking on invalid values is a requirement for the exercise).

However, I'm having issues with where to put the validation functions. Placing it within the impl Ticket kinda makes sense, but I don't want to use my_ticket.validate_title(title). At the same time, it doesn't make sense not to put it in the impl Ticket because that way everyone and their dog would be able to call it.

Sorry if it sounds really dumb but what's the appropriate approach?

ornate drift
slate shell
#

UPDATE: This is an approach I came up with, because I think the validators should be private to the Ticket

pub struct Ticket {
    title: String,
    description: String,
    status: String,
}

impl Ticket {
    pub fn new(title: String, description: String, status: String) -> Ticket {
        Self::validate_title(&title);
        Self::validate_description(&description);
        Self::validate_status(&status);

        Ticket {
            title,
            description,
            status,
        }
    }

    pub fn title(&self) -> &String {
        &self.title
    }

    pub fn description(&self) -> &String {
        &self.description
    }

    pub fn status(&self) -> &String {
        &self.status
    }

    pub fn set_title(&mut self, new_title: String) {
        Self::validate_title(&new_title);
        self.title = new_title;
    }

    pub fn set_description(&mut self, new_description: String) {
        Self::validate_description(&new_description);
        self.description = new_description;
    }

    pub fn set_status(&mut self, new_status: String) {
        Self::validate_status(&new_status);
        self.status = new_status;
    }

    fn validate_title(new_title: &String) {
        if new_title.is_empty() {
            panic!("Title cannot be empty");
        }
        if new_title.len() > 50 {
            panic!("Title cannot be longer than 50 characters");
        }
    }

    fn validate_description(new_description: &String) {
        if new_description.is_empty() {
            panic!("Description cannot be empty");
        }
        if new_description.len() > 500 {
            panic!("Description cannot be longer than 500 characters");
        }
    }

    fn validate_status(new_status: &String) {
        if new_status != "To-Do" && new_status != "In Progress" && new_status != "Done" {
            panic!("Only `To-Do`, `In Progress`, and `Done` statuses are allowed");
        }
    }
}
slate shell
ornate drift
slate shell
#

absolute lightbulb moment! ā¤ļø

#

much appreciated

ornate drift
slate shell
#

yeah, we haven't touched matchers and Results yet but I have a vague idea of how this works

#

you got my question beyond answered šŸ˜

#

thank you again!

#

have a great day

halcyon granite
#

But we work through a few "unidiomatic" iterations first in order to break down the complexity and drive home why the final form is ideal.