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

feature request: conditional build #119

Closed
dimitrilw opened this issue Sep 21, 2023 · 7 comments
Closed

feature request: conditional build #119

dimitrilw opened this issue Sep 21, 2023 · 7 comments

Comments

@dimitrilw
Copy link

I am still a bit new to Rust -- I apologize if this request is a "just do this, noob" type of thing.

I would really like to be able to do something like:

#[derive(TypedBuilder)]
struct Something {
    #[builder(default=42)]
    i: i32,
    #[builder(default="this is Something's default string".to_string())]
    s: String,
}

let something = Something::builder()
    .i(123)
    .map(|b| {
        if global_config.set_string != "" {
            b.s(global_config.set_string.to_string())
        } else if some_vec.len() > 5 {
            b.s("some_vec is longer than 5".to_string())
        } else {
            b // no change
        }
    })
    .build();

If adding this feature is viewed as valuable, then I am willing to learn more about this crate and try to help implement it. I've done it with my home-brew Builder pattern Structs via something like:

impl Something {
    fn map<F>(&self, f: F) -> Something
    where F: Fn(&Something) -> Something {
        f(&self)
    }
}

( Note: that map implementation is not my original code, but I don't recall where to give credit -- one of the Rust books or something, I guess )

While I have not yet worked on anything that does "Derive magic", I guess there is a first time for everything. 😄

@dimitrilw
Copy link
Author

dimitrilw commented Sep 21, 2023

I should add that I am aware I can do post-build manipulation, like:

let mut something = Something::builder()
    .i(123)
    .build();

if global_config.set_string != "" {
    something.s(global_config.set_string.to_string());
} else if some_vec.len() > 5 {
    something.s("some_vec is longer than 5".to_string());
}

Downside: I now have a mutable something, which adds mutation risk.

@idanarye
Copy link
Owner

This map is more like the pipe method from the tap crate - rather than changing an internal value it changes the entire value. So let's use that name, pipe:

.pipe(|b| {
    if global_config.set_string != "" {
        b.s(global_config.set_string.to_string())
    } else if some_vec.len() > 5 {
        b.s("some_vec is longer than 5".to_string())
    } else {
        b // no change
    }
})

Now, the problem with this syntax is that b.s(...) and b have a different type. That's how Typed Builder does its magic. The closure returns one type in the first two branches and another in the third - Rust cannot accept this.

It may be possible to use something like:

.pipe(|b| {
    if global_config.set_string != "" {
        b.s(global_config.set_string.to_string())
    } else if some_vec.len() > 5 {
        b.s("some_vec is longer than 5".to_string())
    } else {
        b.fill_defaults()
    }
})

I think, if we do this right, Rust would infer the generics of fill_defaults() and fill only the types that need to be filled. But my gut tells me this would turn out to be a complicated, hard to teach solution that'll start to creak when generics get involved.

Also, I think that this may be the wrong pattern. Why not combine this with #117 and have something like this?

.s_opt({
    if global_config.set_string != "" {
        Some(global_config.set_string.to_string())
    } else if some_vec.len() > 5 {
        Some("some_vec is longer than 5".to_string())
    } else {
        None // will set some default
    }
})

@dimitrilw
Copy link
Author

I apologize for missing #117 . That's what I get for speed-reading issues. If you don't mind, I will close this one as #117 truly appears to capture the same goal.

Re the branches having different returns:
That's just me not knowing the internals of how this crate makes the Builder pattern. When I've hand-jammed a Builder,
I've really done a light-weight, feature-sparse, and very lazy style... roughly:

#[derive(Default)]
struct Something {
    i: i32,
    s: String,
}

impl Something {
    fn new() -> Something {
        Something::default()
    }
    fn i(&self, val: i32) -> Something {
        self.i = val;
        self
    }
    fn s(&self, val: String) -> Something {
        self.s = val;
        self
    }
    fn map<F>(&self, f: F) -> Something
    where F: Fn(&Something) -> Something {
        f(&self)
    }
}

...please excuse typos & syntax errors in that code block. I'm just speed-jamming some code. Probably missed a ; or more. 😄

@dimitrilw
Copy link
Author

closing

@dimitrilw
Copy link
Author

Thank you for sharing that tap::pipe::Pipe with me, too. I'm still working on "crates I should know"... like, I've been dev'ing for a minute now, but very green to Rust. I'm still learning basics. 😑 ...but I'll get there in time! 😄

@idanarye
Copy link
Owner

No need to apologize (and don't apologize for apologizing. That's cliche). Without this suggestion, I wouldn't get the idea to combine these and #117 would have remained a bypass to strip_option.

@dimitrilw
Copy link
Author

Just channeling my inner-Canadian. 😊

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

2 participants