What pattern or technique can I use in this case?

The following code is a small example of what has been bugging my brain for days.

I'm writing for the first time and by myself a small app that follows the "clean architecture" logic as I have done with other languages (garbage collected).

Can you help me understand what I'm doing wrong and what better pattern I can use (I'm thinking of a builder pattern, but also various setters methods that allow me to change the values when I need it, in several places)?

The biggest convenience I would like to achieve is to be able to use code like this:

if let Some(condition) = &mut input.condition {
    condition.set_subscribed_at_is_nil(true);
} else {
    input.condition = Some(PlayerConditions::new().unset_subscribed_at_is_nil());
}

or better:

input.condition.unwrap_or_default().set_subscribed_at_is_nil(true);

// or maybe input.condition = input.condition.unwrap_or_default().set_subscribed_at_is_nil(true);

if something_happened {
    input.condition.unset_it().set_another(Some("something"));
}

if something_else_happened {
    input.condition.set_another(Some("something")).check_something();
}

What can you suggest?

The code:

/*
[dependencies]
tokio = { version = "1", features = ["full"] }
*/

#[derive(Debug, Default)]
pub struct PlayerConditions {
    id: Option<String>,
    name: Option<String>,
    subscribed_at_is_nil: Option<bool>,
}

impl PlayerConditions {
    pub fn new() -> Self {
        Self::default()
    }
    pub fn is_default(&self) -> bool {
        self.id.is_none() && self.name.is_none() && self.subscribed_at_is_nil.is_none()
    }
    pub fn id(&self) -> &Option<String> {
        &self.id
    }
    pub fn set_id(&mut self, id: Option<String>) -> &mut Self {
        self.id = id;
        self
    }
    pub fn name(&self) -> &Option<String> {
        &self.name
    }
    pub fn set_name(&mut self, name: Option<String>) -> &mut Self {
        self.name = name;
        self
    }
    pub fn subscribed_at_is_nil(&self) -> &Option<bool> {
        &self.subscribed_at_is_nil
    }
    pub fn set_subscribed_at_is_nil(&mut self, subscribed_at_is_nil: Option<bool>) -> &mut Self {
        self.subscribed_at_is_nil = subscribed_at_is_nil;
        self
    }
}

#[derive(Debug, Default)]
pub struct PlayerListInput {
    pub condition: Option<PlayerConditions>,
    // This is not String: is a struct in real code
    pub order: Option<String>,
    // other fields here...
}

#[derive(Debug)]
pub struct DBPlayer {
    pub id: String,
    pub name: Option<String>,
    // For simplicity this is String in this example
    pub subscribed_at: Option<String>,
}

impl DBPlayer {
    fn query_construction<'a>(
        condition: Option<&'a mut PlayerConditions>,
        order: &Option<String>,
    ) -> String {
        // Sometimes I need to change condition here (so I need the mutability of it)

        let query = "SELECT * FROM players".to_string();

        if let Some(condition) = condition {
            if !condition.is_default() {
                // query = condition.add_each_one_to(query);
            }
        }

        if let Some(_order) = &order {
            // add order to query
        }

        query.to_string()
    }

    pub async fn query(
        _db: &str,      // This is not String: is a connection/transaction in real code
        input: &mut PlayerListInput,
    ) -> Vec<DBPlayer> {
        // Sometimes I need to change input here (so I need the mutability of it)

        let _query = Self::query_construction(input.condition.as_mut(), &input.order);

        let players = vec![]; // fetch_them_from_db_using_query_here;

        players
    }
}

async fn players_from_repo(input: &mut PlayerListInput) -> Vec<DBPlayer> {
    // Here I need to change input checking if something is already added, for example:

    if let Some(condition) = &mut input.condition {
        condition.set_subscribed_at_is_nil(Some(true));
    } else {
        input.condition = Some(PlayerConditions::new().set_subscribed_at_is_nil(Some(true)));
    }

    DBPlayer::query( "db_connection", input).await
}

#[tokio::main]
async fn main() {
    let mut input = PlayerListInput::default();

    let players = players_from_repo(&mut input).await;

    // I still need input ownership here
    dbg!(input);

    dbg!(players);
}

The (&mut Self, …) -> &mut Self approach is good for chaining multiple calls to such a method, but one minor disadvantage of &mut Self-reference based API is that it cannot be used directly to create an expression without a helper variable. In other words, you’ll need to put the PlayerConditions::new() into a variable in the else case, mutate it, and finally assign only the result:

else {
    let mut new_condition = PlayerConditions::new();
    new_condition.set_subscribed_at_is_nil(Some(true));
    input.condition = Some(new_condition);
}

One alternative for this particular code however would be to deduplicate the .set_subscribed_at_is_nil(Some(true)) call and handle initialization e.g. via Option::get_or_insert:

// Here I need to change input checking if something is already added, for example:
input
    .condition
    .get_or_insert(PlayerConditions::new())
    .set_subscribed_at_is_nil(Some(true));

A minor thing with this is now that it would always call PlayerConditions::new(), but there’s alternative closure-based API, too:

input
    .condition
    .get_or_insert_with(|| PlayerConditions::new())
    .set_subscribed_at_is_nil(Some(true));

which would only construct the new PlayerConditions if it’s actually needed.

Closures like this can be simplified by just writing the function name, instead of wrapping PlayerConditions::new in a closure with the same signature it already had:

input
    .condition
    .get_or_insert_with(PlayerConditions::new)
    .set_subscribed_at_is_nil(Some(true));
3 Likes

BTW: it's usually better to return Option<&str> instead of &Option<String>, since it will work with the ? operator, and can be used with unwrap_or(…) without allocating.

Use .as_deref() to convert it.

6 Likes

Regarding the Option<&str> topic, I like this video on that general topic / pattern:

4 Likes

you are over using references, it can be simplified a lot if you just pass it by value (make a copy when you need it later).

also, I would consider it a code smell to mutate an argument named input. if a function want to accept optional parameters, create the default value when needed inside the function. if your function need to make sure the parameters are always present, then don't take an Option, instead require the caller to always provide a value (i.e. the caller makes the decision for the default value).

I don't like builder pattern with Option parameters. personally, I would prefer to write it like this:

pub fn with_subscribed_at_is_nil(self, subscribed_at_is_nil: bool) -> Self {
    Self {
        subscribed_at_is_nil: Some(subscribed_at_is_nil),
        ..self
    }
}
pub fn without_subscribed_at_is_nil(self) -> Self {
    Self {
        subscribed_at_is_nil: None,
        ..self
    }
}
condition.with_subscribed_at_is_nil(true);

I don't get it why you need a is_default()? what's the point of is_default() if you already take an Option? can't you just use is_none()?

this logic seems indicate the subscribed_at_is_nil should always be present, and default to true, then why make it optional in the first place?

here's your code which I modified a bit:

note I renamed PlayerConditions to QueryBuilder, and split DBPlayer::query_construction() into QueryBuilder::build() and QueryBuilder::build_with_order(),

all in all, it's not clear what you are really trying to achieve, so I keep most your code intact, but to be honest, I would not normally use builder pattern like this.

typically, builder types are meant to represent incomplete and transient states, and should be converted to the complete type as soon as the incremental configuration phase is done, and domain specific logic should only deal with the complete type, not the transient builders.

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.