v2 MaybeUndefined: tri-state patch fields and upsert payloads - #112
Conversation
b7c8430 to
33ba03b
Compare
EugeneTheDev
left a comment
EugeneTheDev
left a comment
There was a problem hiding this comment.
A few nit comments
| * keeps [previous], [MaybeUndefined.Null] clears it, and [MaybeUndefined.Value] replaces it. | ||
| */ | ||
| @UnstableApi | ||
| public fun <T> MaybeUndefined<T>.update(previous: T?): T? = when (this) { |
There was a problem hiding this comment.
This function confused me for a bit, because it reads the other way. Since you read from left to right, it reads like "this MaybeUndefined is updated with previous value", but in reality it's the opposite. Wdyt swapping the sides of this function to make the intention clearer?
fun <T> T?.update(value: MaybeUndefined<T>): T? There was a problem hiding this comment.
But then it migrates from the MaybeUndefined extensions to T? extension, I'm not sure we want to do that, it's hard to find
There was a problem hiding this comment.
That's true. Wdyt keeping it as an extension on MaybeUndefined then, but renaming to something like takeOrPrevious, to align with standard Kotlin naming conventions?
There was a problem hiding this comment.
Or, sorry, I missed that, similar to the orPrevious below, this one looks like getOrElse from Option monad too. I guess moving it to MaybeUndefined too and calling it getOrElse would align it with Option monad even better
2 participants
No description provided.