garro95/priority-queue

changing priorities via `iter_mut` doesn't work as I expected

jstrong-tios opened this issue · 4 comments

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!

@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

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

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

I published new crate version 1.0.3 that fixes this issue