Skip to content

Commit ec05e7f

Browse files
committed
Refactor to use parent/child notation
In `normalized` we process the threshold policy as well as its child threshold policy and we shadow `subs` - this is all very confusing. Add parent/child terminology and a couple extra local variables in an attempt to make the code more clear. Refactor only, no logic changes.
1 parent bc359c0 commit ec05e7f

File tree

1 file changed

+19
-10
lines changed

1 file changed

+19
-10
lines changed

src/policy/semantic.rs

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -430,19 +430,28 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
430430
let n = subs.len() - unsatisfied_count - trivial_count; // remove all true/false
431431
let m = k.checked_sub(trivial_count).unwrap_or(0); // satisfy all trivial
432432

433-
// m == n denotes `and` and m == 1 denotes `or`
434-
let is_and = m == n;
435-
let is_or = m == 1;
433+
// Call the current threshold "parent" to differentiate it from its "child" threshold below.
434+
let parent_is_and = m == n; // (n, n)-thresh is an AND
435+
let parent_is_or = m == 1; // (1, n)-thresh is an OR
436436

437437
for sub in subs {
438438
match sub.as_ref() {
439439
Policy::Trivial | Policy::Unsatisfiable => {}
440440
Policy::Threshold(ref k, ref subs) => {
441-
match (is_and, is_or) {
442-
(true, true) => ret_subs.push(Arc::clone(&sub)), // m = n = 1, thresh(1,X) type thing.
443-
(true, false) if *k == subs.len() => ret_subs.extend(subs.to_vec()), // and case
444-
(false, true) if *k == 1 => ret_subs.extend(subs.to_vec()), // or case
445-
_ => ret_subs.push(Arc::clone(&sub)),
441+
let child_is_and = *k == subs.len();
442+
let child_is_or = *k == 1;
443+
444+
if parent_is_and && parent_is_or {
445+
// m = n = 1, child must be the non-trivial, non-unsatisfiable node.
446+
ret_subs.push(Arc::clone(&sub));
447+
} else if parent_is_and && child_is_and {
448+
// If both parent and child are ANDs we can flatten them.
449+
subs.iter().for_each(|sub| ret_subs.push(Arc::clone(sub)));
450+
} else if parent_is_or && child_is_or {
451+
// If both parent and child are ORs we can flatten them.
452+
subs.iter().for_each(|sub| ret_subs.push(Arc::clone(sub)));
453+
} else {
454+
ret_subs.push(Arc::clone(&sub));
446455
}
447456
}
448457
_ => ret_subs.push(Arc::clone(&sub)),
@@ -456,9 +465,9 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
456465
} else if ret_subs.len() == 1 {
457466
let policy = ret_subs.pop().unwrap();
458467
(*policy).clone() // I'm lost now, can we try_unwrap still?
459-
} else if is_and {
468+
} else if parent_is_and {
460469
Policy::Threshold(ret_subs.len(), ret_subs)
461-
} else if is_or {
470+
} else if parent_is_or {
462471
Policy::Threshold(1, ret_subs)
463472
} else {
464473
Policy::Threshold(m, ret_subs)

0 commit comments

Comments
 (0)