-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
Adds Observable.sorted method #4264
Conversation
* @return an Observable that emits the items emitted by the source Observable in sorted order | ||
*/ | ||
@Experimental | ||
public final Observable<T> sorted(){ |
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.
wasn't sure to name it sorted or sort, what do u think ?
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.
may as well call it sorted
I think
Current coverage is 84.48% (diff: 100%)@@ 1.x #4264 diff @@
==========================================
Files 268 268
Lines 17477 17477
Methods 0 0
Messages 0 0
Branches 2662 2662
==========================================
+ Hits 14761 14765 +4
+ Misses 1861 1856 -5
- Partials 855 856 +1
|
* Returns an Observable that emits the events emitted by source Observable, in a | ||
* sorted order. Each item emitted by the Observable must implement {@link Comparable} with respect to all | ||
* other items in the sequence. | ||
* <dl> |
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.
Please add a note about long or non-terminating or infinite sources as they may run out of memory or never finish collecting the elements to be sorted.
I'd have implemented a single operator for it but this will also do. |
public class OperatorSortedTest { | ||
|
||
@Test | ||
public void testSortedList() { |
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.
testSorted
Looks generally okay but I can't tell how often this shortcut is needed. I hope for some community feedback on this. |
testSubscriber.assertNotCompleted(); | ||
} | ||
|
||
private final class NonComparable{ |
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: static
👍 |
1 similar comment
👍 |
This PR Observable.sorted Method, I hope it addresses this #4263 correctly.