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

changing priorities via iter_mut doesn't work as I expected #26

Closed
jstrong-tios opened this issue Nov 19, 2020 · 4 comments
Closed

changing priorities via iter_mut doesn't work as I expected #26

jstrong-tios opened this issue Nov 19, 2020 · 4 comments
Assignees
Labels

Comments

@jstrong-tios
Copy link

hello,

Thank you for your work on this library!

Based on the documentation, I had thought that changing the priorities of items in the course of iterating over them via iter_mut would update their priority rankings in the queue. However, unless I am missing something, that does not seem to be the case.

Here is a simple example of what I mean in the form of a test:

#[cfg(test)]
mod tests {
    use std::time::*;

    #[test]
    fn verify_how_priority_queue_works() {
        let mut queue: priority_queue::PriorityQueue<&'static str, Duration> = Default::default();

        queue.push("a", Duration::new(0, 0));
        queue.push("b", Duration::new(0, 1));
        assert_eq!(queue.peek().unwrap().0, &"b");
        for (k, v) in queue.iter_mut() {
            if k == &"a" {
                *v = Duration::new(0, 2);
            }
        }
        assert_eq!(queue.peek().unwrap().0, &"a"); // this assertion fails
    }
}

the peek call instead returns the "b" item.

if this test is modified to use queue.change_priority("a", Duration::new(0, 2)) rather than assigning to the priority as it is above, the return value of peek is "a" as expected.

The docs say:

It's not an error, instead, to modify the priorities, because the heap will be rebuilt once the IterMut goes out of scope.

Based on this, I had thought that changing the value of the priority when iterating over the queue via iter_mut would work the same as change_priority.

Could you tell me if this a bug, or a misunderstanding of how `PriorityQueue works?

what would be a good way to update all of the priorities at once?

thank you for your help on this!

@garro95 garro95 added the bug label Nov 20, 2020
@garro95 garro95 self-assigned this Nov 20, 2020
@garro95
Copy link
Owner

garro95 commented Nov 20, 2020

@jstrong-tios Can I add that test to the test suite of this crate? It will be distributed as the rest of the library under the terms of the GNU LGPLv3 or the MPLv2, at the user opinion

@garro95
Copy link
Owner

garro95 commented Nov 20, 2020

I am grateful to you because you found a long standing, very little bug that was there from at least 2018.

@jstrong-tios
Copy link
Author

sure, you can use that test, np. I'm glad this was helpful to you.

@garro95
Copy link
Owner

garro95 commented Nov 20, 2020

I published new crate version 1.0.3 that fixes this issue

@garro95 garro95 closed this as completed Nov 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants