Skip to content

Commit 5449986

Browse files
committed
fix(dijkstra)!: remove unneeded partial cost parameter
The `dijkstra_reach` function used a successors function which not only took the node whose successors are seeked, but also the cost so far to reach this node. This is an oversight, as this information can be retrieved from the iterator returned by `dijkstra_reach`. The user is free to stop iterating if a maximum cost is reached, so there is no loss in functionality.
1 parent d1733ff commit 5449986

2 files changed

Lines changed: 7 additions & 18 deletions

File tree

src/directed/dijkstra.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ impl<N, C, FN, IN> Iterator for DijkstraReachable<N, C, FN>
348348
where
349349
N: Eq + Hash + Clone,
350350
C: Zero + Ord + Copy + Hash,
351-
FN: FnMut(&N, C) -> IN,
351+
FN: FnMut(&N) -> IN,
352352
IN: IntoIterator<Item = (N, C)>,
353353
{
354354
type Item = DijkstraReachableItem<N, C>;
@@ -367,7 +367,7 @@ where
367367
parent: self.parents.get_index(*parent_index).map(|x| x.0.clone()),
368368
total_cost,
369369
});
370-
(self.successors)(node, total_cost)
370+
(self.successors)(node)
371371
};
372372
for (successor, move_cost) in successors {
373373
let new_cost = cost + move_cost;
@@ -404,14 +404,13 @@ where
404404
/// Visit all nodes that are reachable from a start node. The node
405405
/// will be visited in order of cost, with the closest nodes first.
406406
///
407-
/// The `successors` function receives the current node and the best
408-
/// cost up to this node, and returns an iterator of successors
409-
/// associated with their move cost.
407+
/// The `successors` function receives the current node, and returns
408+
/// an iterator of successors associated with their move cost.
410409
pub fn dijkstra_reach<N, C, FN, IN>(start: &N, successors: FN) -> DijkstraReachable<N, C, FN>
411410
where
412411
N: Eq + Hash + Clone,
413412
C: Zero + Ord + Copy,
414-
FN: FnMut(&N, C) -> IN,
413+
FN: FnMut(&N) -> IN,
415414
IN: IntoIterator<Item = (N, C)>,
416415
{
417416
let mut to_see = BinaryHeap::new();

tests/dijkstra-reach.rs

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use std::collections::HashMap;
44

55
#[test]
66
fn dijkstra_reach_numbers() {
7-
let reach = dijkstra_reach(&0, |prev, _| vec![(prev + 1, 1), (prev * 2, *prev)])
7+
let reach = dijkstra_reach(&0, |prev| vec![(prev + 1, 1), (prev * 2, *prev)])
88
.take_while(|x| x.total_cost < 100)
99
.collect_vec();
1010
// the total cost should equal to the node's value, since the starting node is 0 and the cost to reach a successor node is equal to the increase in the node's value
@@ -30,13 +30,7 @@ fn dijkstra_reach_graph() {
3030
graph.insert("B", vec![("C", 2)]);
3131
graph.insert("C", vec![]);
3232

33-
let mut costs = HashMap::new();
34-
35-
let reach = dijkstra_reach(&"A", |prev, cost| {
36-
costs.insert(*prev, cost);
37-
graph[prev].clone()
38-
})
39-
.collect_vec();
33+
let reach = dijkstra_reach(&"A", |prev| graph[prev].clone()).collect_vec();
4034

4135
// need to make sure that a node won't be returned twice when a better path is found after the first candidate
4236
assert!(
@@ -59,8 +53,4 @@ fn dijkstra_reach_graph() {
5953
},
6054
]
6155
);
62-
63-
for item in reach {
64-
assert!(item.total_cost == costs[item.node]);
65-
}
6656
}

0 commit comments

Comments
 (0)