Skip to content

Commit cb4c037

Browse files
committed
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 8622ee6 commit cb4c037

File tree

1 file changed

+45
-23
lines changed

1 file changed

+45
-23
lines changed

src/miniscript/types/extra_props.rs

Lines changed: 45 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -809,13 +809,8 @@ impl Property for ExtData {
809809
);
810810
}
811811

812-
// We sort by [satisfaction cost - dissatisfaction cost] to make a worst-case (the most
813-
// costy satisfaction are satisfied, the most costy dissatisfactions are dissatisfied)
814-
// sum of the cost by iterating through the sorted vector *backward*.
815-
stack_elem_count_sat_vec.sort_by(|a, b| {
816-
a.0.map(|x| a.1.map(|y| x as isize - y as isize))
817-
.cmp(&b.0.map(|x| b.1.map(|y| x as isize - y as isize)))
818-
});
812+
stack_elem_count_sat_vec.sort_by(sat_minus_option_dissat);
813+
// Sum of the cost by iterating through the sorted vector *backward*.
819814
for (i, &(x, y)) in stack_elem_count_sat_vec.iter().rev().enumerate() {
820815
stack_elem_count_sat = if i <= k {
821816
x.and_then(|x| stack_elem_count_sat.map(|count| count + x))
@@ -824,11 +819,7 @@ impl Property for ExtData {
824819
};
825820
}
826821

827-
// Same logic as above
828-
exec_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-
});
822+
exec_stack_elem_count_sat_vec.sort_by(sat_minus_option_dissat);
832823
for (i, &(x, y)) in exec_stack_elem_count_sat_vec.iter().rev().enumerate() {
833824
exec_stack_elem_count_sat = if i <= k {
834825
opt_max(exec_stack_elem_count_sat, x)
@@ -837,14 +828,8 @@ impl Property for ExtData {
837828
};
838829
}
839830

840-
// Same for the size cost. A bit more intricated as we need to account for both the witness
841-
// and scriptSig cost, so we end up with a tuple of Options of tuples. We use the witness
842-
// cost (first element of the mentioned tuple) here.
843831
// FIXME: Maybe make the ExtData struct aware of Ctx and add a one_cost() method here ?
844-
max_sat_size_vec.sort_by(|a, b| {
845-
a.0.map(|x| a.1.map(|y| x.0 as isize - y.0 as isize))
846-
.cmp(&b.0.map(|x| b.1.map(|y| x.0 as isize - y.0 as isize)))
847-
});
832+
max_sat_size_vec.sort_by(sat_minus_dissat_witness);
848833
for (i, &(x, y)) in max_sat_size_vec.iter().enumerate() {
849834
max_sat_size = if i <= k {
850835
x.and_then(|x| max_sat_size.map(|(w, s)| (w + x.0, s + x.1)))
@@ -853,10 +838,7 @@ impl Property for ExtData {
853838
};
854839
}
855840

856-
ops_count_sat_vec.sort_by(|a, b| {
857-
a.0.map(|x| x as isize - a.1 as isize)
858-
.cmp(&b.0.map(|x| x as isize - b.1 as isize))
859-
});
841+
ops_count_sat_vec.sort_by(sat_minus_dissat);
860842
for (i, &(x, y)) in ops_count_sat_vec.iter().enumerate() {
861843
op_count_sat = if i <= k {
862844
opt_add(op_count_sat, x)
@@ -1021,6 +1003,46 @@ impl Property for ExtData {
10211003
}
10221004
}
10231005

1006+
// Function to pass to sort_by. Sort by (satisfaction cost - dissatisfaction cost).
1007+
//
1008+
// We sort by (satisfaction cost - dissatisfaction cost) to make a worst-case (the most
1009+
// costy satisfactions are satisfied, the most costy dissatisfactions are dissatisfied).
1010+
//
1011+
// Args are of form: (<count_sat>, <count_dissat>)
1012+
fn sat_minus_dissat<'r, 's>(
1013+
a: &'r (Option<usize>, usize),
1014+
b: &'s (Option<usize>, usize),
1015+
) -> std::cmp::Ordering {
1016+
a.0.map(|x| x as isize - a.1 as isize)
1017+
.cmp(&b.0.map(|x| x as isize - b.1 as isize))
1018+
}
1019+
1020+
// Function to pass to sort_by. Sort by (satisfaction cost - dissatisfaction cost).
1021+
//
1022+
// We sort by (satisfaction cost - dissatisfaction cost) to make a worst-case (the most
1023+
// costy satisfactions are satisfied, the most costy dissatisfactions are dissatisfied).
1024+
//
1025+
// Args are of form: (<count_sat>, <count_dissat>)
1026+
fn sat_minus_option_dissat<'r, 's>(
1027+
a: &'r (Option<usize>, Option<usize>),
1028+
b: &'s (Option<usize>, Option<usize>),
1029+
) -> std::cmp::Ordering {
1030+
a.0.map(|x| a.1.map(|y| x as isize - y as isize))
1031+
.cmp(&b.0.map(|x| b.1.map(|y| x as isize - y as isize)))
1032+
}
1033+
1034+
// Function to pass to sort_by. Sort by (satisfaction cost - dissatisfaction cost) of cost of witness.
1035+
//
1036+
// Args are of form: (<max_sat_size>, <count_dissat_size>)
1037+
// max_[dis]sat_size of form: (<cost_of_witness>, <cost_of_sciptsig>)
1038+
fn sat_minus_dissat_witness<'r, 's>(
1039+
a: &'r (Option<(usize, usize)>, Option<(usize, usize)>),
1040+
b: &'s (Option<(usize, usize)>, Option<(usize, usize)>),
1041+
) -> std::cmp::Ordering {
1042+
a.0.map(|x| a.1.map(|y| x.0 as isize - y.0 as isize))
1043+
.cmp(&b.0.map(|x| b.1.map(|y| x.0 as isize - y.0 as isize)))
1044+
}
1045+
10241046
// Returns Some(max(x,y)) is both x and y are Some. Otherwise, return none
10251047
fn opt_max<T: Ord>(a: Option<T>, b: Option<T>) -> Option<T> {
10261048
if let (Some(x), Some(y)) = (a, b) {

0 commit comments

Comments
 (0)