-
Notifications
You must be signed in to change notification settings - Fork 124
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
Implement Uni.onItem().ifNotNull() #111
Conversation
.onItem().ifNotNull().apply(String::toUpperCase) | ||
.onItem().ifNull().continueWith("yolo!") | ||
.await().indefinitely(); | ||
assertThat(r).isEqualTo("yolo!"); |
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.
I must be not grokking as I don't get how this would pass.
uni
has null
and therefore will return yolo!
on subscription.
So I expected that r
would return YOLO!
and not yolo!
.
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.
The uni produces null
, so ifNotNull().apply(String::toUpperCase)
is not called, and null
is propagated downstream. As a consequence .onItem().ifNull().continueWith("yolo!")
is called, yolo!
is emitted.
Does that make sense?
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.
But if you switch the 2 lines, it will become YOLO!
, right?
(Because it's a pipeline, even though it really doesn't look like one.)
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.
Exactly.
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.
Ah, so are you saying onItem().ifNull()
in the uni
is never called because the previous line already consumed the onItem()
?
If so, how would that line ever get executed?
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.
Each line is a stage, so there is an item emitted on every line.
Uni.createFrom().item(() -> null); // emits `null`
.onItem().ifNotNull().apply(String::toUpperCase) // skipped because the item is null, so emits (another) `null`
.onItem().ifNull().continueWith("yolo!") // called because the received item is `null`, and emits "yolol!"
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.
Maybe my confusion is that the code is actually duplicated? In that the same onItem()
calls are added to the same uni
which means it's a single chain of 4 possible outcomes, as opposed to 2 chains with 2 possible outcomes each.
Am I reading that right now?
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.
So, yes the code is duplicated, as the first one (does nothing - no one subscribes) is just there for the snippet inclusion in the doc.
The second one is actually checking that it does the right thing.
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.
If I'm understanding, you could remove lines 33 and 34 and just do uni.await()
right, as the onItem()
entries are still in the Uni?
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.
No, every line returns a new Uni. So if line 27 we affect a new variable, and reuse it, then yes.
This group checks if the item is not
null
. If the item isnull
default results are provided (null
, emptyMulti
...)This PR also contains various cosmetic fixes.
Related to #109