Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Builder pattern + methods #47

Open
Emilgardis opened this issue Dec 6, 2020 · 4 comments
Open

Builder pattern + methods #47

Emilgardis opened this issue Dec 6, 2020 · 4 comments

Comments

@Emilgardis
Copy link
Member

Currently, this crate uses the builder pattern to assure a stable API i.e https://github.com/Emilgardis/twitch_api2/blob/e1ba9faff6c72b92f271a888b480a016c334b82a/src/helix/channels.rs#L82-L88

I decided to do it this way due to Twitch not guaranteeing a stable API themselves on documented items, so marking structs as #[non_exhaustive] and using a builder seems better.

However, sometimes this makes things harder to use, as typed-builder does not exactly make structs more accessible.

While this is done for stability, I feel like some structs don't lend themselves to the builder pattern, like the above example.

I want to solve this somehow, without sacrificing API stability.

To do that I think what needs to be done is creating methods for common usages of requests.

Example:

impl GetChannelInformationRequest {
    pub fn get_channel(broadcaster: String) -> GetChannelInformationRequest {
        GetChannelInformationRequest::builder().broadcaster_id(broadcaster).build()
    }
}

or for https://github.com/Emilgardis/twitch_api2/blob/e1ba9faff6c72b92f271a888b480a016c334b82a/src/helix/games.rs#L59-L68

impl GetGamesRequest  {
    pub fn get_game(id: types::CategoryId) -> GetGamesRequest {
        GetGamesRequest::builder().id(vec![id]).build()
    }
}

There is the crate derive_builder that could help also.

@Emilgardis Emilgardis pinned this issue Dec 17, 2020
@ModProg
Copy link
Contributor

ModProg commented Apr 17, 2021

I want to add to this that it makes it really difficult to optionally set some fields.

This is the simplest I could do this, but Maybe there is also a better way, I'm not seeing:

pub struct ChannelInformation<'a> {
    title: Option<&'a str>,
    language: Option<&'a str>,
    game: Option<&'a str>,
}
impl ChannelInformation<'_> {
    fn to_modify_body(&self) -> ModifyChannelInformationBody {
        match (self.title, self.game) {
            (Some(t), Some(g)) => ModifyChannelInformationBody::builder()
                .title(t)
                .game_id(g)
                .broadcaster_language(self.language.map(ToOwned::to_owned))
                .build(),
            (Some(t), None) => ModifyChannelInformationBody::builder()
                .title(t)
                .broadcaster_language(self.language.map(ToOwned::to_owned))
                .build(),
            (None, Some(g)) => ModifyChannelInformationBody::builder()
                .game_id(g)
                .broadcaster_language(self.language.map(ToOwned::to_owned))
                .build(),
            (None, None) => ModifyChannelInformationBody::builder()
                .broadcaster_language(self.language.map(ToOwned::to_owned))
                .build(),
        }
    }
}

@Emilgardis
Copy link
Member Author

this is due to strip_option, should consider not using it at all and just rely on into

@spikespaz
Copy link

How does typed_builder currently work? The builder type is private, the build function is crossed-out and RA fills in a single parameter channel_points_custom_reward_redemption_add_v1_builder_error_missing_required_field_broadcaster_user_id. I can't figure out how to finalize the builder.

@Emilgardis
Copy link
Member Author

Emilgardis commented Apr 3, 2024

@spikespaz the typed_builder crate creates a builder that throws a compile error if setters are missing, the parameter you get contains the missing setter, you need to add broadcaster_user_id. Anyway, I'd recommend using ChannelPointsCustomRewardRedemptionAddV1::broadcaster_user_id(...) instead which is what this issue is about. Looks like I forgot to link the pr that created them: #262

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants