Fixed the blocking/non-blocking first #520

zsxwing · 2013-11-25T03:28:23Z

Hi, this PR fixed the last problem in #423.

Observable.first in RxJava is same as Observable.FirstAsync in Rx.Net, which is a non-blocking operator. When the source sequence is empty, it will emit an IllegalArgumentException.
Observable.takeFirst is an alias of take(1). If the source sequence is empty, it will return an empty sequence.
BlockingObservable.first in RxJava is same as Observable.First in Rx.Net, which is a blocking operator. When the source sequence is empty, it will emit an IllegalArgumentException.
also added BlockingObservable.firstOrDefault.

Here I attached my test codes for Rx.Net.

            Console.WriteLine("===Test FirstAsync");
            {
                IObservable<int> obs = Observable.Range(1, 4).FirstAsync();
                obs.Subscribe(
                    x => Console.WriteLine("FirstAsync: " + x),
                    e => Console.WriteLine("FirstAsync: " + e),
                    () => Console.WriteLine("FirstAsync completed")
                    );
            }
            {
                IObservable<int> obs = Observable.Empty<int>().FirstAsync();
                obs.Subscribe(
                    x => Console.WriteLine("FirstAsync with empty: " + x),
                    e => Console.WriteLine("FirstAsync with empty: " + e),
                    () => Console.WriteLine("FirstAsync with empty completed")
                    );
            }
            Console.WriteLine("===Test Take(1)");
            {
                IObservable<int> obs = Observable.Range(1, 4).Take(1);
                obs.Subscribe(
                    x => Console.WriteLine("Take(1): " + x),
                    e => Console.WriteLine("Take(1): " + e),
                    () => Console.WriteLine("Take(1) completed")
                    );
            }
            {
                IObservable<int> obs = Observable.Empty<int>().Take(1);
                obs.Subscribe(
                    x => Console.WriteLine("Take(1) with empty: " + x),
                    e => Console.WriteLine("Take(1) with empty: " + e),
                    () => Console.WriteLine("Take(1) with empty completed")
                    );
            }
            Console.WriteLine("===Test First(1)");
            {
                int value = Observable.Range(1, 4).First();
                Console.WriteLine("First: " + value);
            }
            {
                try
                {
                    int value = Observable.Empty<int>().First();
                }
                catch(Exception e) {
                    Console.WriteLine("First with empty: " + e);
                }
            }

The output is:

===Test FirstAsync
FirstAsync: 1
FirstAsync completed
FirstAsync with empty: System.InvalidOperationException: Sequence contains no elements.
===Test Take(1)
Take(1): 1
Take(1) completed
Take(1) with empty completed
===Test First(1)
First: 1
First with empty: System.InvalidOperationException: Sequence contains no elements.
...

cloudbees-pull-request-builder · 2013-11-25T03:33:16Z

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

cloudbees-pull-request-builder · 2013-11-26T05:38:24Z

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

benjchristensen · 2013-11-26T19:44:23Z

This one is on the list for review...

cloudbees-pull-request-builder · 2013-11-27T06:49:51Z

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

benjchristensen · 2013-12-03T22:28:46Z

The changes look good ... basically correcting misunderstandings of first , take, firstAsync etc.

@headinthebox Can you weigh in on this?

cloudbees-pull-request-builder · 2013-12-05T03:02:40Z

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

zsxwing · 2013-12-05T06:49:43Z

I rebased it on the latest codes.

By the way, @benjchristensen , if you have time, could you close the enhancement issues which have already finished. It's very helpful to find out what is remaining on the TODO list.
I checked the opening issues today. Here is the issues I think can be closed:
#14 #15 #22 #23 #24 #25 #32 #38 #39 #41 #56 #61 #63 #68 #69 #78 #79 #80 #82 #83 #88 #90 #91 #98 #99 #100

zsxwing · 2013-12-05T07:01:41Z

Sorry that my comment messed up the issues. Is there some way to delete the references in other issues?

headinthebox · 2013-12-06T02:02:27Z

Quick question, is it worthwhile to have an alias for take(1) that is actually longer takeFirst()?
The reason the operation existed was because we wanted first.
So let's remove all trivial ones.

zsxwing · 2013-12-06T02:39:09Z

I'm OK to remove takeFirst. But I worry about breaking the existed public API.

benjchristensen · 2013-12-06T03:02:26Z

We can mark it deprecated right now if we want to remove it.

On Dec 6, 2013, at 1:39 PM, Shixiong Zhu [email protected] wrote:

I'm OK to remove takeFirst. But I worry about breaking the existed public API.

—
Reply to this email directly or view it on GitHub.

zsxwing · 2013-12-06T03:25:51Z

rxjava-core/src/main/java/rx/Observable.java

     */
    public Observable<T> takeFirst(Func1<? super T, Boolean> predicate) {
-        return first(predicate);
+        return skipWhile(not(predicate)).take(1);


For this overload of takeFirst, I suppose we should reserve it.

cloudbees-pull-request-builder · 2013-12-06T03:27:12Z

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

zsxwing · 2013-12-09T01:50:16Z

Rebased it. Any further suggestion?

cloudbees-pull-request-builder · 2013-12-09T01:53:33Z

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

cloudbees-pull-request-builder · 2013-12-18T03:12:41Z

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

cloudbees-pull-request-builder · 2013-12-19T15:15:17Z

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

zsxwing · 2013-12-19T15:16:43Z

I implemented the non-blocking single and singeOrDefault and used them to reimplement the blocking/nonblocking single, singleOrDefault, first, firstOrDefault, last, lastOrDefault. Here are the differences between these operators:

RxJava operators	more than one items in the input sequence	one item in the input sequence	empty sequence	method name in Rx.Net
Observable.single	IllegalArgumentException	the single item	IllegalArgumentException	singleAsync
BlockingObservable.single	IllegalArgumentException	the single item	IllegalArgumentException	single
Observable.singleOrDefault	IllegalArgumentException	the single item	the default value	singleOrDefaultAsync
BlockingObservable.singleOrDefault	IllegalArgumentException	the single item	the default value	singleOrDefault
Observable.first	the first item	the first item	IllegalArgumentException	firstAsync
BlockingObservable.first	the first item	the first item	IllegalArgumentException	first
Observable.firstOrDefault	the first item	the first item	the default value	firstOrDefaultAsync
BlockingObservable.firstOrDefault	the first item	the first item	the default value	firstOrDefault
Observable.last	the last item	the last item	IllegalArgumentException	lastAsync
BlockingObservable.last	the last item	the last item	IllegalArgumentException	last
Observable.lastOrDefault	the last item	the last item	the default value	lastOrDefaultAsync
BlockingObservable.lastOrDefault	the last item	the last item	the default value	lastOrDefault

I changed public Observable<T> firstOrDefault(Func1<? super T, Boolean> predicate, T defaultValue) to public Observable<T> firstOrDefault(T defaultValue, Func1<? super T, Boolean> predicate) to keep these operators consistent. However, it breaks the previous API.

cloudbees-pull-request-builder · 2013-12-19T15:43:49Z

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

…irstOrDefault, last, lastOrDefault

cloudbees-pull-request-builder · 2013-12-20T03:51:12Z

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

akarnokd · 2013-12-20T08:00:31Z

rxjava-core/src/main/java/rx/operators/OperationSingle.java

+
+                    private volatile T value;
+                    private volatile boolean isEmpty = true;
+                    private volatile boolean hasTooManyElemenets;


I think these don't need to be volatiles.

Removed "volatile". Thanks!

cloudbees-pull-request-builder · 2013-12-20T09:59:51Z

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

benjchristensen · 2013-12-23T20:43:35Z

Excellent table explaining what this is all did!

@DavidMGross Can you validate all of our documentation on these operators aligns with that table above?

Fixed the blocking/non-blocking first

benjchristensen · 2013-12-23T20:51:19Z

Thank you @zsxwing. This is a good set of fixes.

DavidMGross · 2013-12-27T00:04:15Z

I've looked over the Wiki docs & javadocs and both seem to be aligned with
the table now. I've also added the table itself as an appendix to the
BlockingObservable wiki page, here:

https://github.com/Netflix/RxJava/wiki/Blocking-Observable-Operators#appendix-similar-blocking-and-non-blocking-operators

On Mon, Dec 23, 2013 at 12:51 PM, Ben Christensen
[email protected]:

Thank you @zsxwing https://github.com/zsxwing. This is a good set of
fixes.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/520#issuecomment-31143126
.

David M. Gross
PLP Consulting

zsxwing · 2013-12-27T01:54:00Z

Great work. Thanks, @DavidMGross

Fixed the blocking/non-blocking first

zsxwing reviewed Dec 6, 2013
View reviewed changes

zsxwing mentioned this pull request Dec 18, 2013

More operators in Rx.Net #634

Closed

53 tasks

zsxwing added 5 commits December 20, 2013 11:44

Fixed the blocking/non-blocking first

0608155

Add MSDN links

21e7523

Mark takeFirst deprecated

e20613b

Implement the blocking/non-blocking single, singleOrDefault, first, f…

96064c3

…irstOrDefault, last, lastOrDefault

Fix the unit tests in groovy and kotlin

bfb9cd1

akarnokd reviewed Dec 20, 2013
View reviewed changes

Remove volatile

a1191a2

benjchristensen added a commit that referenced this pull request Dec 23, 2013

Merge pull request #520 from zsxwing/first

cfb8f5f

Fixed the blocking/non-blocking first

benjchristensen merged commit cfb8f5f into ReactiveX:master Dec 23, 2013

zsxwing deleted the first branch December 24, 2013 02:45

benjchristensen mentioned this pull request Jan 7, 2014

Version 0.16.0 Release Candidate #601

Closed

rickbw pushed a commit to rickbw/RxJava that referenced this pull request Jan 9, 2014

Merge pull request ReactiveX#520 from zsxwing/first

ce734f7

Fixed the blocking/non-blocking first

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed the blocking/non-blocking first #520

Fixed the blocking/non-blocking first #520

zsxwing commented Nov 25, 2013

cloudbees-pull-request-builder commented Nov 25, 2013

cloudbees-pull-request-builder commented Nov 26, 2013

benjchristensen commented Nov 26, 2013

cloudbees-pull-request-builder commented Nov 27, 2013

benjchristensen commented Dec 3, 2013

cloudbees-pull-request-builder commented Dec 5, 2013

zsxwing commented Dec 5, 2013

zsxwing commented Dec 5, 2013

headinthebox commented Dec 6, 2013

zsxwing commented Dec 6, 2013

benjchristensen commented Dec 6, 2013

zsxwing Dec 6, 2013

cloudbees-pull-request-builder commented Dec 6, 2013

zsxwing commented Dec 9, 2013

cloudbees-pull-request-builder commented Dec 9, 2013

cloudbees-pull-request-builder commented Dec 18, 2013

cloudbees-pull-request-builder commented Dec 19, 2013

zsxwing commented Dec 19, 2013

cloudbees-pull-request-builder commented Dec 19, 2013

cloudbees-pull-request-builder commented Dec 20, 2013

akarnokd Dec 20, 2013

zsxwing Dec 20, 2013

cloudbees-pull-request-builder commented Dec 20, 2013

benjchristensen commented Dec 23, 2013

benjchristensen commented Dec 23, 2013

DavidMGross commented Dec 27, 2013

zsxwing commented Dec 27, 2013

Fixed the blocking/non-blocking first #520

Fixed the blocking/non-blocking first #520

Conversation

zsxwing commented Nov 25, 2013

cloudbees-pull-request-builder commented Nov 25, 2013

cloudbees-pull-request-builder commented Nov 26, 2013

benjchristensen commented Nov 26, 2013

cloudbees-pull-request-builder commented Nov 27, 2013

benjchristensen commented Dec 3, 2013

cloudbees-pull-request-builder commented Dec 5, 2013

zsxwing commented Dec 5, 2013

zsxwing commented Dec 5, 2013

headinthebox commented Dec 6, 2013

zsxwing commented Dec 6, 2013

benjchristensen commented Dec 6, 2013

zsxwing Dec 6, 2013

Choose a reason for hiding this comment

cloudbees-pull-request-builder commented Dec 6, 2013

zsxwing commented Dec 9, 2013

cloudbees-pull-request-builder commented Dec 9, 2013

cloudbees-pull-request-builder commented Dec 18, 2013

cloudbees-pull-request-builder commented Dec 19, 2013

zsxwing commented Dec 19, 2013

cloudbees-pull-request-builder commented Dec 19, 2013

cloudbees-pull-request-builder commented Dec 20, 2013

akarnokd Dec 20, 2013

Choose a reason for hiding this comment

zsxwing Dec 20, 2013

Choose a reason for hiding this comment

cloudbees-pull-request-builder commented Dec 20, 2013

benjchristensen commented Dec 23, 2013

benjchristensen commented Dec 23, 2013

DavidMGross commented Dec 27, 2013

zsxwing commented Dec 27, 2013