refactor: return DerivationResult from derive_at_index#946
Conversation
trevarj
force-pushed
the
derive_at_index-ergo
branch
2 times, most recently
from
05cbda9 to
7431081
Compare
trevarj
changed the title
derive_at_indexDerivationResult from derive_at_index
|
Yeah, concept ACK. I was skeptical of this design at first but I think it leads to pretty nice-looking code. |
|
Is |
trevarj
force-pushed
the
derive_at_index-ergo
branch
from
7431081 to
84dd4df
Compare
this is the new function that was recently added in #911. |
Things stay for at least two versions, but maybe (much) longer if they never get in the way of anything. |
| /// The descriptor has no wildcard. Carries the original for fallback. | ||
| WithoutWildcard(Descriptor<DescriptorPublicKey>), | ||
| /// An error other than NoWildcard. | ||
| Error(NonDefiniteKeyError), |
There was a problem hiding this comment.
Either doc or variant name needs changing, right?
There was a problem hiding this comment.
It's referring to NonDefiniteKeyError::NoWildcard, so I think it's correct still.
There was a problem hiding this comment.
Updated all NoWildcard references to full name - NonDefiniteKeyError::NoWildcard
There was a problem hiding this comment.
Ah my bad, hard to get good reviews huh.
| /// Result of [`Descriptor::derive_at_index`]. | ||
| /// | ||
| /// When the source descriptor has no wildcard, | ||
| /// [`or_fallback`](DerivationResult::or_fallback) recovers the definite |
There was a problem hiding this comment.
| /// [`or_fallback`](DerivationResult::or_fallback) recovers the definite | |
| /// [`or_fallback`](DerivationResult::or_fallback) recovers the original |
I think?
There was a problem hiding this comment.
It returns the DefiniteDescriptor derived from the original descriptor (Descriptor<DescriptorPublicKey>)
This allows a caller of `derive_at_index` to fall back on the original descriptor (as a `Descriptor<DefiniteDescriptorKey>`) without having to `match` or `if-let` on the `Result` when there is no wildcard. We introduce a new result type called `DerivationResult` to handle this, but since the `Try` trait is not stabilized in our MSRV we cannot have `?` and must use `into_result()?` instead.
trevarj
force-pushed
the
derive_at_index-ergo
branch
from
84dd4df to
a15ce35
Compare
3 participants
This allows a caller of
derive_at_indexto fall back on the originaldescriptor (as a
Descriptor<DefiniteDescriptorKey>) without having tomatchor
if-leton theResultwhen there is no wildcard.We introduce a new result type called
DerivationResultto handle this, butsince the
Trytrait is not stabilized in our MSRV we cannot have?and mustuse
into_result()?instead.Closes #918
I ultimately settled on
or_fallbackcause I feel that it reads nicely withderive_at_index(0).Not sure if I really like this solution when we can't use
Tryand then justderive_at_index(0)?