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

Implement StreamMap::get to be able to obtain a reference to a specific stream #5131

Open
j-a-m-l opened this issue Oct 27, 2022 · 2 comments
Labels
A-tokio-stream Area: The tokio-stream crate C-feature-request Category: A feature request. M-stream Module: tokio/stream S-breaking-change A breaking change that requires manual coordination to be released.

Comments

@j-a-m-l
Copy link

j-a-m-l commented Oct 27, 2022

Is your feature request related to a problem? Please describe.
Currently is not possible to get a reference to a stream in StreamMap.
I'd like to contribute to change that if you are OK with my approach.

Describe the solution you'd like
Although I've not tried it yet, a possible implementation is:

    pub fn get<Q: ?Sized>(&self, key: &Q) -> Option<&V>
    where
        K: Borrow<Q>,
        Q: Hash + Eq,
    {
        self.iter().find(|(k, _)| k == &key).map(|(_, value)| value)
    }

Describe alternatives you've considered
The proposed solution mimics HashMap::get and it is similar to StreamMap::contains_key.

Additional context
I'm using something like this in my project:

struct MyStreams {
  streams: StreamMap<String, MyStream>
}

impl Streams {
    pub fn get_stream(&self, key: &str) -> Option<&MyStream> {
        self.streams.iter().find(|(k, _)| k == &key).map(|(_, value)| value)
    }
}
@j-a-m-l j-a-m-l added A-tokio Area: The main tokio crate C-feature-request Category: A feature request. labels Oct 27, 2022
@Darksonn Darksonn added M-stream Module: tokio/stream A-tokio-stream Area: The tokio-stream crate and removed A-tokio Area: The main tokio crate labels Oct 29, 2022
@Darksonn
Copy link
Contributor

I am reluctant to add this under the current implementation due to its running time, but a rewrite that uses a HashMap would make this easy to provide.

@j-a-m-l
Copy link
Author

j-a-m-l commented Oct 30, 2022

It is clearly stated in the documentation that «StreamMap works best with a “smallish” number of streams as all entries are scanned on insert, remove, and polling». So, this is something that it's not going to surprise any developer that uses it.

I understand your concern about the performance of the current implementation, but I'm not sure why adding this method could affect in any negative way the current implementation. I don't even think that it could constraint the future HashMap based implementation in any way.

My idea was creating a PR with that function, documentation and tests, maybe even implementing other similar functions in other PRs just for fun, but I doubt that I could dedicate time to rewrite the internals, with all the possible edge cases, etc. during the next months which, BTW, are quite busy for me already.

@maminrayej maminrayej added the S-breaking-change A breaking change that requires manual coordination to be released. label Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio-stream Area: The tokio-stream crate C-feature-request Category: A feature request. M-stream Module: tokio/stream S-breaking-change A breaking change that requires manual coordination to be released.
Projects
None yet
Development

No branches or pull requests

3 participants