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

Tracking Issue for BinaryHeap::contains and BinaryHeap::remove #82001

Closed
3 tasks
billyrieger opened this issue Feb 11, 2021 · 3 comments
Closed
3 tasks

Tracking Issue for BinaryHeap::contains and BinaryHeap::remove #82001

billyrieger opened this issue Feb 11, 2021 · 3 comments
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@billyrieger
Copy link
Contributor

billyrieger commented Feb 11, 2021

Feature gate: #![feature(binary_heap_contains_remove)]

This is a tracking issue for BinaryHeap::contains and BinaryHeap::remove. These methods are missing from the BinaryHeap API and both are common operations that one would expect to have for any general-purpose container.

Previous discussion: #66724.

Public API

impl<T: Ord> BinaryHeap<T> {
    pub fn contains<Q: ?Sized>(&self, item: &Q) -> bool
    where
        T: Borrow<Q>,
        Q: PartialEq;

    pub fn remove<Q: ?Sized>(&self, item: &Q) -> Option<T>
    where
        T: Borrow<Q>,
        Q: PartialEq;
}

Steps / History

Unresolved Questions

  • What is the best way to check if an item exists in a BinaryHeap?
@billyrieger billyrieger added C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Feb 11, 2021
@goffrie
Copy link
Contributor

goffrie commented Feb 13, 2021

This seems like a trap in that you would usually want these to run in less-than-linear time, but they don't. e.g. the standard Dijkstra/A* applications (as in the linked discussion) definitely want sublinear access, otherwise it defeats the point.

That being said Vec::remove also exists and takes linear time.

@est31
Copy link
Member

est31 commented Feb 17, 2021

@goffrie good point. Maybe the docs added by #82002 should include this as a caveat and point to using a BTreeSet if you need to do such an operation more often?

@m-ou-se
Copy link
Member

m-ou-se commented Mar 3, 2021

I'm closing this for now since it's not clear #82002 will be merged. We can re-open it later if needed.

(Next time please open a tracking issue only after the addition was approved by a libs reviewer. Thanks!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants