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

Some usability improvements + tests/docs #12

Merged
merged 7 commits into from Mar 19, 2019
Merged

Conversation

kstrafe
Copy link
Contributor

@kstrafe kstrafe commented Mar 17, 2019

I'm using this crate to implement a timer-based reactor for callbacks, unfortunately std::time::Instant does not impl Default so I can't create a default object of PriorityQueue.

There is no good reason for P to be Default, so this patch series implements Default manually by relegating to new, which works for all P: Ord. Deriving default appears to add a hidden requirement for P: Default.

kstrafe and others added 7 commits March 17, 2019 16:51
Implementing default manually avoids the hidden requirement that P: Default,
which is not implemented for some std types.

The use case is to allow the priority queue to be easily added to
already-default structures that derive default themselves as:

        #[derive(Default)]
        struct MyState {
            ...
            pub pq: PriorityQueue<i32, Instant>,
        }

Without needing to `impl Default` for the entire struct.

In my specific case, I use the priority queue as a callback mechanism
using time instants from std::time::Instant. If the top elements are
behind the `now()` time, we run these instants and put them back onto
the queue with an added interval (for repeating timers).
When we change a priority, I do not want to change the contents of the
element (do not basically want to pop and then push the new thing),
because sometimes we have elements (like callbacks in this test) that we
want to keep.

This patch implements a test that confirms this.
The documentation contains a reference to reversing priority but that's
not very useful when you don't know how to use it. This test allows
users to see how you use `Reverse`, and confirms that it works.
@kstrafe
Copy link
Contributor Author

kstrafe commented Mar 17, 2019

The benchmarks seem to only work on nightly due to the unstable feature(test).

@garro95
Copy link
Owner

garro95 commented Mar 19, 2019

I don't like it does not work in stable any more

@garro95 garro95 merged commit fd1d15e into garro95:master Mar 19, 2019
@kstrafe
Copy link
Contributor Author

kstrafe commented Mar 19, 2019

@garro95
bench: Add benchmarks for pushing/popping

if you remove that one it should work. Otherwise we can use a #[cfg] or something to conditionally enable benchmarking

@garro95
Copy link
Owner

garro95 commented Mar 19, 2019

The latter is what I did

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

Successfully merging this pull request may close these issues.

None yet

2 participants