Followup to #895: rename dissat_sat function and use struct rather than tuple for return values#930
Open
apoelstra wants to merge 2 commits intorust-bitcoin:masterfrom
Open
Followup to #895: rename dissat_sat function and use struct rather than tuple for return values#930apoelstra wants to merge 2 commits intorust-bitcoin:masterfrom
dissat_sat function and use struct rather than tuple for return values#930apoelstra wants to merge 2 commits intorust-bitcoin:masterfrom
Conversation
It is too easy to get the satisfactions and dissatisfactions confused. Use a struct with named fields. Curiously, outside of this module dissatisfactions are never used, so we can keep the `dissat` field private. (Well, this is a bit of a sketchy thing to say -- dissatisfactions **are** used by the Satisfaction::thresh and thresh_mall functions, but those functions take dissats and sats as independent Vecs they can manipulate, rather than taking a SatDissat struct.)
The name of the function no longer implies its return value (which is a struct rather than a tuple that needs to be carefully ordered). So change the name to be consistent with the name of the module.
Member
Author
|
On 5097641 successfully ran local tests |
trevarj
approved these changes
Apr 23, 2026
Comment on lines
+486
to
+489
| let (dissats, sats): (Vec<_>, Vec<_>) = sat_dissats | ||
| .into_iter() | ||
| .map(|SatDissat { dissat, sat }| (dissat, sat)) | ||
| .unzip(); |
Contributor
There was a problem hiding this comment.
let (dissats, sats): (Vec<_>, Vec<_>) = stack
.drain(stack.len() - thresh.n()..)
.map(|SatDissat { dissat, sat }| (dissat, sat))
.unzip();
| } | ||
| } | ||
|
|
||
| /// The (dissatisfaction, satisfaction) pair for an `older` fragment. |
Contributor
There was a problem hiding this comment.
minor nit, can ignore: Find and replace on "(dissatisfaction, satisfaction) pair" to just "satisfaction and dissatisfaction" or "SatDissat" in the doc comments.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes naming more consistent and reduces confusion caused by having a tuple with two entries of the same type.