-
Notifications
You must be signed in to change notification settings - Fork 831
Refactor logical operators #808
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
In preparation for refactoring `read_scriptbool` add passing unit tests.
Refactor and simplify the logical operators in `read_scriptbool`. Refactor only, no logic changes.
Refactor with the aim of simplifying `is_p2kh`. This function is covered sufficiently by current unit tests. Refactor only, no logic changes.
In an effort to make code containing multi-line logical AND clearer to read put the operator at the start of the line.
Refactor with the aim of making the code easier to read. Code path is covered by current unit tests. Refactor only, no logic changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 4b6e866
I do find these easier to follow than their versions in the other PR :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 4b6e866
Simplify `read_scriptbool` by doing: - Use `split_last` to get at the last element - Mask the last byte against ^0x80 instead of using two equality statements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beautiful!
ACK df7bb03
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK df7bb03
In an effort to make the code clearer and more explicit, do various refactorings around logical operators. Each done as a separate patch to ease review and limit scope of discussion.
Based on review of #806