From e84877e0b0080b84601578b1de301594b73b5da9 Mon Sep 17 00:00:00 2001 From: Wenjie Li Date: Sun, 6 Nov 2022 21:20:37 -0800 Subject: [PATCH 1/4] make change_priority API consistent --- src/double_priority_queue/mod.rs | 15 +++++++++++---- src/priority_queue/mod.rs | 15 +++++++++++---- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/double_priority_queue/mod.rs b/src/double_priority_queue/mod.rs index 6e9b228..6b54e49 100644 --- a/src/double_priority_queue/mod.rs +++ b/src/double_priority_queue/mod.rs @@ -472,17 +472,24 @@ where } /// Change the priority of an Item using the provided function. + /// or `None` if the item wasn't in the queue. + /// + /// The argument `item` is only used for lookup, and is not used to overwrite the item's data + /// in the priority queue. + /// /// The item is found in **O(1)** thanks to the hash table. /// The operation is performed in **O(log(N))** time (worst case). - pub fn change_priority_by(&mut self, item: &Q, priority_setter: F) + pub fn change_priority_by(&mut self, item: &Q, priority_setter: F) -> Option<()> where I: Borrow, Q: Eq + Hash, F: FnOnce(&mut P), { - if let Some(pos) = self.store.change_priority_by(item, priority_setter) { - self.up_heapify(pos); - } + self.store + .change_priority_by(item, priority_setter) + .map(|pos| { + self.up_heapify(pos); + }) } /// Get the priority of an item, or `None`, if the item is not in the queue diff --git a/src/priority_queue/mod.rs b/src/priority_queue/mod.rs index 2dc4e25..c2449c4 100644 --- a/src/priority_queue/mod.rs +++ b/src/priority_queue/mod.rs @@ -406,17 +406,24 @@ where } /// Change the priority of an Item using the provided function. + /// or `None` if the item wasn't in the queue. + /// + /// The argument `item` is only used for lookup, and is not used to overwrite the item's data + /// in the priority queue. + /// /// The item is found in **O(1)** thanks to the hash table. /// The operation is performed in **O(log(N))** time (worst case). - pub fn change_priority_by(&mut self, item: &Q, priority_setter: F) + pub fn change_priority_by(&mut self, item: &Q, priority_setter: F) -> Option<()> where I: Borrow, Q: Eq + Hash, F: FnOnce(&mut P), { - if let Some(pos) = self.store.change_priority_by(item, priority_setter) { - self.up_heapify(pos); - } + self.store + .change_priority_by(item, priority_setter) + .map(|pos| { + self.up_heapify(pos); + }) } /// Get the priority of an item, or `None`, if the item is not in the queue From 9f4accc6952118c2eea0d3bc9e6bf91d13b693e2 Mon Sep 17 00:00:00 2001 From: Wenjie Li Date: Wed, 9 Nov 2022 11:07:35 -0800 Subject: [PATCH 2/4] Change return type for change_priority_by from Option to bool --- src/double_priority_queue/mod.rs | 6 +++--- src/priority_queue/mod.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/double_priority_queue/mod.rs b/src/double_priority_queue/mod.rs index 6b54e49..a1780df 100644 --- a/src/double_priority_queue/mod.rs +++ b/src/double_priority_queue/mod.rs @@ -472,14 +472,14 @@ where } /// Change the priority of an Item using the provided function. - /// or `None` if the item wasn't in the queue. + /// Return a boolean value where `true` means the item was in the queue and update was successful /// /// The argument `item` is only used for lookup, and is not used to overwrite the item's data /// in the priority queue. /// /// The item is found in **O(1)** thanks to the hash table. /// The operation is performed in **O(log(N))** time (worst case). - pub fn change_priority_by(&mut self, item: &Q, priority_setter: F) -> Option<()> + pub fn change_priority_by(&mut self, item: &Q, priority_setter: F) -> bool where I: Borrow, Q: Eq + Hash, @@ -489,7 +489,7 @@ where .change_priority_by(item, priority_setter) .map(|pos| { self.up_heapify(pos); - }) + }).is_some() } /// Get the priority of an item, or `None`, if the item is not in the queue diff --git a/src/priority_queue/mod.rs b/src/priority_queue/mod.rs index c2449c4..7562cd8 100644 --- a/src/priority_queue/mod.rs +++ b/src/priority_queue/mod.rs @@ -406,14 +406,14 @@ where } /// Change the priority of an Item using the provided function. - /// or `None` if the item wasn't in the queue. + /// Return a boolean value where `true` means the item was in the queue and update was successful /// /// The argument `item` is only used for lookup, and is not used to overwrite the item's data /// in the priority queue. /// /// The item is found in **O(1)** thanks to the hash table. /// The operation is performed in **O(log(N))** time (worst case). - pub fn change_priority_by(&mut self, item: &Q, priority_setter: F) -> Option<()> + pub fn change_priority_by(&mut self, item: &Q, priority_setter: F) -> bool where I: Borrow, Q: Eq + Hash, @@ -423,7 +423,7 @@ where .change_priority_by(item, priority_setter) .map(|pos| { self.up_heapify(pos); - }) + }).is_some() } /// Get the priority of an item, or `None`, if the item is not in the queue From eb2abf5cdece4187e9cd3c3dda80630e78013588 Mon Sep 17 00:00:00 2001 From: Wenjie Li Date: Wed, 9 Nov 2022 11:14:58 -0800 Subject: [PATCH 3/4] Run cargo fmt --- src/double_priority_queue/mod.rs | 3 ++- src/priority_queue/mod.rs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/double_priority_queue/mod.rs b/src/double_priority_queue/mod.rs index a1780df..f92b155 100644 --- a/src/double_priority_queue/mod.rs +++ b/src/double_priority_queue/mod.rs @@ -489,7 +489,8 @@ where .change_priority_by(item, priority_setter) .map(|pos| { self.up_heapify(pos); - }).is_some() + }) + .is_some() } /// Get the priority of an item, or `None`, if the item is not in the queue diff --git a/src/priority_queue/mod.rs b/src/priority_queue/mod.rs index 7562cd8..20b4950 100644 --- a/src/priority_queue/mod.rs +++ b/src/priority_queue/mod.rs @@ -423,7 +423,8 @@ where .change_priority_by(item, priority_setter) .map(|pos| { self.up_heapify(pos); - }).is_some() + }) + .is_some() } /// Get the priority of an item, or `None`, if the item is not in the queue From 252550e5b323705edf6fdc1d80d0f2034b94529e Mon Sep 17 00:00:00 2001 From: Wenjie Li Date: Wed, 9 Nov 2022 11:23:48 -0800 Subject: [PATCH 4/4] Add test coverage for API change --- tests/double_priority_queue.rs | 4 ++-- tests/priority_queue.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/double_priority_queue.rs b/tests/double_priority_queue.rs index db66eb4..77a84fc 100644 --- a/tests/double_priority_queue.rs +++ b/tests/double_priority_queue.rs @@ -259,9 +259,9 @@ mod doublepq_tests { let v = vec![("a", 1), ("b", 2), ("f", 7), ("g", 6), ("h", 5)]; let mut pq: DoublePriorityQueue<_, _> = DoublePriorityQueue::from_iter(v.into_iter()); - pq.change_priority_by("z", |z| *z += 8); + assert!(!pq.change_priority_by("z", |z| *z += 8)); - pq.change_priority_by("b", |b| *b += 8); + assert!(pq.change_priority_by("b", |b| *b += 8)); assert_eq!( pq.into_descending_sorted_vec().as_slice(), &["b", "f", "g", "h", "a"] diff --git a/tests/priority_queue.rs b/tests/priority_queue.rs index d5e1fe8..73cceba 100644 --- a/tests/priority_queue.rs +++ b/tests/priority_queue.rs @@ -249,7 +249,7 @@ mod pqueue_tests { let v = vec![("a", 1), ("b", 2), ("f", 7), ("g", 6), ("h", 5)]; let mut pq: PriorityQueue<_, _> = PriorityQueue::from_iter(v.into_iter()); - pq.change_priority_by("b", |b| *b += 8); + assert!(pq.change_priority_by("b", |b| *b += 8)); assert_eq!(pq.into_sorted_vec().as_slice(), &["b", "f", "g", "h", "a"]); }