Skip to content

BinaryHeap example should show how to make a min-heap. #58174

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

Closed
forrestthewoods opened this issue Feb 5, 2019 · 6 comments · Fixed by #60451
Closed

BinaryHeap example should show how to make a min-heap. #58174

forrestthewoods opened this issue Feb 5, 2019 · 6 comments · Fixed by #60451
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@forrestthewoods
Copy link

std::collections::BinaryHeap is a max-heap. A very common use case for users will be to make a min-heap. The basic example should include a min-heap.

@hellow554
Copy link
Contributor

@jonas-schievink jonas-schievink added C-enhancement Category: An issue proposing an enhancement or a PR with one. E-help-wanted Call for participation: Help is requested to fix this issue. A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. and removed E-help-wanted Call for participation: Help is requested to fix this issue. labels Apr 21, 2019
@rasendubi
Copy link
Contributor

Added an example in #60451

@HernandoR
Copy link

I would like to point out that the current tutorial of making min-heap introduces unnecessary Some(Reverse(v))) to the heap.

Is there any alternative I could approach to? or is it possible to introduce a compare function to binary heap?

@hellow554
Copy link
Contributor

I would like to point out that the current tutorial of making min-heap introduces unnecessary Some(Reverse(v))) to the heap.

Not sure what you mean by unnecessary here. Can you explain?

Is there any alternative I could approach to? or is it possible to introduce a compare function to binary heap?

Using a newtype is the way to use a custom compare function. By using Reverse you simply reverse the original compare function which is most often easier than completely rewriting your own.

@HernandoR
Copy link

HernandoR commented Mar 3, 2025

I would like to point out that the current tutorial of making min-heap introduces unnecessary Some(Reverse(v))) to the heap.

Not sure what you mean by unnecessary here. Can you explain?

As the tutorial shows, if I would like a min-heap based on binary heap, i need to unwrap the Reverse by

let Some(Reverse(n))=heap.pop() else {return ()};
// in compare with
let Some(n)=heap.pop() else {return ()};

It feels like an unnecessary gap for the differance of min or max heap. In my point of view, i would expect identical interface for the getter/setter since both are using the heap. How the heap is orderd should only be considered on the builder of that heap. i.e.

//not working, just to show the concept of decide the order at building a heap
let mut rev_heap = BinaryHeap::<Reverse>::new();

// user, setting
rev_heap.push(1);
rev_heap.push(5);
rev_heap.push(2);

// user, getting
// If we pop these scores now, they should come back in the reverse order.
assert_eq!(rev_heap.pop(), Some(1));
let Some(n)=rev_heap.pop() else {return ()};
assert_eq!(n,2);

To conclude, i think the general use case is to set the order when defining the heap, rather than when the first time using it.

ps. I could fully understand that we would like to provide a minimal and stable interface for such a basic data structure. And let people decide what they actually want. But we could discuss if there's a better way, couldn't we?

@hellow554
Copy link
Contributor

Alright, I get your point. Let me try to clarify some things.

It seems to me that you're new to Rust. If you questions feel free to ask them on ULRO
That's the appropriate place, since this Tracker is used to track issues.

In my point of view, i would expect identical interface for the getter/setter since both are using the heap.

They are! Reverse has nothing to do with the heap. It's just a struct like Option or MyStruct. There's nothing special about it. Reverse just reversed the PartialOrd Trait, nothing more, nothing less

How the heap is orderd should only be considered on the builder of that heap

That's a possible approach, but not the one taken by the Rust language. Here the struct that is put in the Heap says how ordering should be done by using the PartialOrd / Ord Trait. This makes Things easier, because you dont have to store a closure in the heap struct.

let mut rev_heap = BinaryHeap::<Reverse>::new();

The correct one would be: BinaryHeap::<Reverse<i32>>::new();

To conclude, i think the general use case is to set the order when defining the heap, rather than when the first time using it.

That's not correct. As said, the struct inside the heap (in your case i32) defines the ordering. Not "the first time using it".

I hope this makes it clearer to you. If not, feel free to open a new topic on the users forum

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants