Skip to content
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

Added: BO.Latest, fixed: BO.next, BO.mostRecent, BO.toIterable #626

Merged
merged 3 commits into from
Dec 23, 2013

Conversation

akarnokd
Copy link
Member

  • Implemented Latest (Issue Operator: Latest #59)
  • Fixed Next and MostRecent to connect to the source observable only when the iterator is asked for.
  • Fixed ToIterable.next() not properly handling the completed status (repeated calls to just the next() would simply block on the empty queue).

@cloudbees-pull-request-builder

RxJava-pull-requests #557 SUCCESS
This pull request looks good

boolean oHasValue;
Notification.Kind oKind;
T oValue;
Throwable oError;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using materialize to pack the all kinds of messages is better

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know. One less memory allocation seemed a good saving.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using materialize and Notification makes the code clearer, doesn't it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, okay, I'll change things around.

@cloudbees-pull-request-builder

RxJava-pull-requests #559 FAILURE
Looks like there's a problem with this pull request

@cloudbees-pull-request-builder

RxJava-pull-requests #560 SUCCESS
This pull request looks good

@DavidMGross
Copy link
Collaborator

Does this capture the essentials, more-or-less, or should I draw up a more
complicated example with hasNext() calls?

https://raw.github.com/wiki/Netflix/RxJava/images/rx-operators/B.next.png
https://raw.github.com/wiki/Netflix/RxJava/images/rx-operators/B.latest.png

On Tue, Dec 17, 2013 at 1:11 AM, CloudBees pull request builder plugin <
[email protected]> wrote:

RxJava-pull-requests #560https://netflixoss.ci.cloudbees.com/job/RxJava-pull-requests/560/SUCCESS

This pull request looks good

Reply to this email directly or view it on GitHubhttps://github.com//pull/626#issuecomment-30735813
.

David M. Gross
PLP Consulting

@akarnokd
Copy link
Member Author

I imagined something like these: next, latest

@DavidMGross
Copy link
Collaborator

Hmmm... It looks like I don't understand latest(). I figured the second
next() call in your latest() illustration would return the pink item, since
that is the latest unreturned item at the time the next() call is made.
Instead, you have it skip that one, and the red item emitted after the
next() call is made, and instead pick up the green item... I don't
understand the logic that governs this decision.

On Tue, Dec 17, 2013 at 12:32 PM, akarnokd [email protected] wrote:

I imagined something like these: nexthttps://docs.google.com/file/d/0B4T7ZW3brESKYllwckdxcG11TTg,
latest https://docs.google.com/file/d/0B4T7ZW3brESKSkt6cjUtSEhUX2s


Reply to this email directly or view it on GitHubhttps://github.com//pull/626#issuecomment-30787987
.

David M. Gross
PLP Consulting

@zsxwing
Copy link
Member

zsxwing commented Dec 18, 2013

@DavidMGross I think your diagram is right.

@zsxwing
Copy link
Member

zsxwing commented Dec 18, 2013

The codes look correct and much clearer.

@akarnokd
Copy link
Member Author

@DavidMGross this is due to the internal race between placing a value in the oNotif and removing one. So even if the next() has woken up, it still could be preempted, and a new Notification placed by onNext overwrites the previous value. Technically, the red item wakes up next(), but the green arrives just in time so it is emitted.

@zsxwing zsxwing mentioned this pull request Dec 18, 2013
53 tasks
@benjchristensen
Copy link
Member

Thank you for these fixes.

benjchristensen added a commit that referenced this pull request Dec 23, 2013
Added: BO.Latest, fixed: BO.next, BO.mostRecent, BO.toIterable
@benjchristensen benjchristensen merged commit f757324 into ReactiveX:master Dec 23, 2013
rickbw pushed a commit to rickbw/RxJava that referenced this pull request Jan 9, 2014
Added: BO.Latest, fixed: BO.next, BO.mostRecent, BO.toIterable
@akarnokd akarnokd deleted the OperationLatestAndFixes branch January 13, 2014 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants