Description
get_operation_count_by_status (lines 462-496) and get_operations_by_status (lines 499-540) each contain an identical match status { ... } block that computes whether an Op matches a given OperationStatus:
let matches = match status {
OperationStatus::Cancelled => op.cancelled,
OperationStatus::Executed => op.executed,
OperationStatus::Expired => {
!op.executed && !op.cancelled && now > op.eta + op.grace_period_seconds
}
OperationStatus::Ready => {
!op.executed
&& !op.cancelled
&& now >= op.eta
&& now <= op.eta + op.grace_period_seconds
}
OperationStatus::Queued => !op.executed && !op.cancelled && now < op.eta,
};
This ~11-line block is copy-pasted verbatim between the two functions.
Impact
Any future change to how a status is determined (e.g., adjusting the Ready/Expired boundary) requires remembering to update both copies in lockstep, which is easy to get wrong and already has to stay consistent with the similar logic in get_operation_status (lines 402-412).
Suggested fix
Extract a private helper, e.g.:
fn op_matches_status(op: &Op, status: &OperationStatus, now: u64) -> bool {
match status {
OperationStatus::Cancelled => op.cancelled,
OperationStatus::Executed => op.executed,
OperationStatus::Expired => {
!op.executed && !op.cancelled && now > op.eta + op.grace_period_seconds
}
OperationStatus::Ready => {
!op.executed && !op.cancelled && now >= op.eta && now <= op.eta + op.grace_period_seconds
}
OperationStatus::Queued => !op.executed && !op.cancelled && now < op.eta,
}
}
and call Self::op_matches_status(&op, &status, now) from both get_operation_count_by_status and get_operations_by_status.
Description
get_operation_count_by_status(lines 462-496) andget_operations_by_status(lines 499-540) each contain an identicalmatch status { ... }block that computes whether anOpmatches a givenOperationStatus:This ~11-line block is copy-pasted verbatim between the two functions.
Impact
Any future change to how a status is determined (e.g., adjusting the
Ready/Expiredboundary) requires remembering to update both copies in lockstep, which is easy to get wrong and already has to stay consistent with the similar logic inget_operation_status(lines 402-412).Suggested fix
Extract a private helper, e.g.:
and call
Self::op_matches_status(&op, &status, now)from bothget_operation_count_by_statusandget_operations_by_status.