Skip to content

Commit dd19e19

Browse files
tchardingjoemphilips
authored andcommitted
Refactor out sort algorithm
The `threshold` function is a candidate for refactoring because it contains code at multiple layers of abstraction as well as duplicate code. Refactor out the sorting closures into three, very similar, functions. Refactor only, no logic changes.
1 parent 764548a commit dd19e19

File tree

1 file changed

+45
-23
lines changed

1 file changed

+45
-23
lines changed

src/miniscript/types/extra_props.rs

+45-23
Original file line numberDiff line numberDiff line change
@@ -822,13 +822,8 @@ impl Property for ExtData {
822822
);
823823
}
824824

825-
// We sort by [satisfaction cost - dissatisfaction cost] to make a worst-case (the most
826-
// costy satisfaction are satisfied, the most costy dissatisfactions are dissatisfied)
827-
// sum of the cost by iterating through the sorted vector *backward*.
828-
stack_elem_count_sat_vec.sort_by(|a, b| {
829-
a.0.map(|x| a.1.map(|y| x as isize - y as isize))
830-
.cmp(&b.0.map(|x| b.1.map(|y| x as isize - y as isize)))
831-
});
825+
stack_elem_count_sat_vec.sort_by(sat_minus_option_dissat);
826+
// Sum of the cost by iterating through the sorted vector *backward*.
832827
for (i, &(x, y)) in stack_elem_count_sat_vec.iter().rev().enumerate() {
833828
stack_elem_count_sat = if i <= k {
834829
x.and_then(|x| stack_elem_count_sat.map(|count| count + x))
@@ -837,11 +832,7 @@ impl Property for ExtData {
837832
};
838833
}
839834

840-
// Same logic as above
841-
exec_stack_elem_count_sat_vec.sort_by(|a, b| {
842-
a.0.map(|x| a.1.map(|y| x as isize - y as isize))
843-
.cmp(&b.0.map(|x| b.1.map(|y| x as isize - y as isize)))
844-
});
835+
exec_stack_elem_count_sat_vec.sort_by(sat_minus_option_dissat);
845836
for (i, &(x, y)) in exec_stack_elem_count_sat_vec.iter().rev().enumerate() {
846837
exec_stack_elem_count_sat = if i <= k {
847838
opt_max(exec_stack_elem_count_sat, x)
@@ -850,14 +841,8 @@ impl Property for ExtData {
850841
};
851842
}
852843

853-
// Same for the size cost. A bit more intricated as we need to account for both the witness
854-
// and scriptSig cost, so we end up with a tuple of Options of tuples. We use the witness
855-
// cost (first element of the mentioned tuple) here.
856844
// FIXME: Maybe make the ExtData struct aware of Ctx and add a one_cost() method here ?
857-
max_sat_size_vec.sort_by(|a, b| {
858-
a.0.map(|x| a.1.map(|y| x.0 as isize - y.0 as isize))
859-
.cmp(&b.0.map(|x| b.1.map(|y| x.0 as isize - y.0 as isize)))
860-
});
845+
max_sat_size_vec.sort_by(sat_minus_dissat_witness);
861846
for (i, &(x, y)) in max_sat_size_vec.iter().enumerate() {
862847
max_sat_size = if i <= k {
863848
x.and_then(|x| max_sat_size.map(|(w, s)| (w + x.0, s + x.1)))
@@ -866,10 +851,7 @@ impl Property for ExtData {
866851
};
867852
}
868853

869-
ops_count_sat_vec.sort_by(|a, b| {
870-
a.0.map(|x| x as isize - a.1 as isize)
871-
.cmp(&b.0.map(|x| x as isize - b.1 as isize))
872-
});
854+
ops_count_sat_vec.sort_by(sat_minus_dissat);
873855
for (i, &(x, y)) in ops_count_sat_vec.iter().enumerate() {
874856
op_count_sat = if i <= k {
875857
opt_add(op_count_sat, x)
@@ -1034,6 +1016,46 @@ impl Property for ExtData {
10341016
}
10351017
}
10361018

1019+
// Function to pass to sort_by. Sort by (satisfaction cost - dissatisfaction cost).
1020+
//
1021+
// We sort by (satisfaction cost - dissatisfaction cost) to make a worst-case (the most
1022+
// costy satisfactions are satisfied, the most costy dissatisfactions are dissatisfied).
1023+
//
1024+
// Args are of form: (<count_sat>, <count_dissat>)
1025+
fn sat_minus_dissat<'r, 's>(
1026+
a: &'r (Option<usize>, usize),
1027+
b: &'s (Option<usize>, usize),
1028+
) -> std::cmp::Ordering {
1029+
a.0.map(|x| x as isize - a.1 as isize)
1030+
.cmp(&b.0.map(|x| x as isize - b.1 as isize))
1031+
}
1032+
1033+
// Function to pass to sort_by. Sort by (satisfaction cost - dissatisfaction cost).
1034+
//
1035+
// We sort by (satisfaction cost - dissatisfaction cost) to make a worst-case (the most
1036+
// costy satisfactions are satisfied, the most costy dissatisfactions are dissatisfied).
1037+
//
1038+
// Args are of form: (<count_sat>, <count_dissat>)
1039+
fn sat_minus_option_dissat<'r, 's>(
1040+
a: &'r (Option<usize>, Option<usize>),
1041+
b: &'s (Option<usize>, Option<usize>),
1042+
) -> std::cmp::Ordering {
1043+
a.0.map(|x| a.1.map(|y| x as isize - y as isize))
1044+
.cmp(&b.0.map(|x| b.1.map(|y| x as isize - y as isize)))
1045+
}
1046+
1047+
// Function to pass to sort_by. Sort by (satisfaction cost - dissatisfaction cost) of cost of witness.
1048+
//
1049+
// Args are of form: (<max_sat_size>, <count_dissat_size>)
1050+
// max_[dis]sat_size of form: (<cost_of_witness>, <cost_of_sciptsig>)
1051+
fn sat_minus_dissat_witness<'r, 's>(
1052+
a: &'r (Option<(usize, usize)>, Option<(usize, usize)>),
1053+
b: &'s (Option<(usize, usize)>, Option<(usize, usize)>),
1054+
) -> std::cmp::Ordering {
1055+
a.0.map(|x| a.1.map(|y| x.0 as isize - y.0 as isize))
1056+
.cmp(&b.0.map(|x| b.1.map(|y| x.0 as isize - y.0 as isize)))
1057+
}
1058+
10371059
// Returns Some(max(x,y)) is both x and y are Some. Otherwise, return none
10381060
fn opt_max<T: Ord>(a: Option<T>, b: Option<T>) -> Option<T> {
10391061
if let (Some(x), Some(y)) = (a, b) {

0 commit comments

Comments
 (0)