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

initialize SharedState without Default #660

Open
avh4 opened this issue Jul 21, 2024 · 2 comments
Open

initialize SharedState without Default #660

avh4 opened this issue Jul 21, 2024 · 2 comments

Comments

@avh4
Copy link

avh4 commented Jul 21, 2024

Would you be open to changing the SharedState API not require use of Default for setting the initial value? (This would also make it more like the RwLock API.) In my case, I am using SharedState to wrap an object which I instantiate with a constructor called load because it needs to do filesystem IO. I could make it implement Default, but I'd prefer not to have a Default implementation that does IO because I want it to be clear that creating new instances of this object should be done sparingly. So I'd like to have some way to create a SharedState by passing in an initial value that I've already created.

I'd propose an API that better mirror's RwLock's API:

impl<Data> SharedState<Data> {
    pub const fn new(t: Data) -> SharedState<Data>;
}

impl<Data> Default for SharedState<Data> {
    fn default() -> SharedState<Data>;
}

Maybe a problem with that proposal that SharedState::default() is not const which I'm gathering is maybe a use case you want to make easy for Relm users? If that's the case, maybe also adding pub const fn from_default() -> SharedState<Data> would solve that? (I'm pretty new to Rust, and not very familiar with const and with when lazy initialized statics are needed.)

An alternative (or additional) proposal would be to leave the existing SharedState::new, and add impl<Data> From<Data> for SharedState<Data>. Imo, this still leaves SharedState::new as being a bit weird for someone familiar with RwLock, but it would solve my need, and would avoid a breaking change to SharedState::new.

If there's agreement on an API change, I expect I'd be able to make a PR to implement it if that's helpful.

@AaronErhardt
Copy link
Member

I'm open for this change, in fact it would be relatively easy to add this. Shared state is based on once_cell::Lazy and the only change would be adding a method that passes a function instead of Default::default() to Lazy::new().

@avh4
Copy link
Author

avh4 commented Jul 22, 2024

👍 assuming no one else gets to it before then, I expect to have time to make the PR this coming weekend.

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