-
Notifications
You must be signed in to change notification settings - Fork 437
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
catch for EffectScope #2746
catch for EffectScope #2746
Changes from 5 commits
4f113bc
822c89b
39247b9
8c31fe3
e9b86d2
f18db85
533f0fc
4958b48
9693ab6
c19e720
b77ce50
c08c236
ba8e4f3
1e1f63e
b1197af
93564cf
f3907a4
81b0711
08cf9e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -240,3 +240,17 @@ public suspend fun <R, B : Any> EffectScope<R>.ensureNotNull(value: B?, shift: ( | |
contract { returns() implies (value != null) } | ||
return value ?: shift(shift()) | ||
} | ||
|
||
/** | ||
* Catch any possible `shift`ed outcome from [action], and transform it with [handler]. | ||
* This reads like a `catch` block, but for `EffectScope`. | ||
* | ||
* The [handler] may `shift` into a different `ErrorScope`, which is useful to | ||
* simulate re-throwing of exceptions. | ||
*/ | ||
public suspend fun<E, R, A> EffectScope<E>.catch( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needs to be an extension due to variance? This duplicates behaviour with I am absolutely up for improving these APIs, and renaming them but we should probably avoid duplicated APIs as much as possible to keep the API surface reasonable. That being said this signatures reminds me a lot of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For me the question is how much of the API should be exposed via There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. About not being an extension, I just wrote it directly like that, although it could also be part of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This also an alternative, using the // in EffectScope interface public suspend fun <B> Effect<R, B>.catch(recover: suspend EffectScope<R>.(@UnsafeVariance R) -> B): B =
fold({ recover(it) }, ::identity) we can also have the Effect be a parameter, if this is a better encoding |
||
f: suspend EffectScope<R>.() -> A, | ||
recover: suspend EffectScope<E>.(R) -> A | ||
): A = effect(f).fold( | ||
recover = { recover(it) }, transform = { x -> x } | ||
) |
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.
NIT: Recover first, operation last.
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.
Also for a DSL like this? I know it's the convention for
bimap
,fold
etc but to me it feels counter-intuitive here since this models an operation liketry { } catch { }
but for typed errors.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.
Looking at things like Flow's
catch
, I think that the convention is action, then recovery.