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

Adds Observable.sorted method #4264

Merged
merged 5 commits into from
Aug 2, 2016

Conversation

SherifMakhlouf
Copy link

This PR Observable.sorted Method, I hope it addresses this #4263 correctly.

* @return an Observable that emits the items emitted by the source Observable in sorted order
*/
@Experimental
public final Observable<T> sorted(){
Copy link
Author

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 ?

Copy link
Collaborator

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

@codecov-io
Copy link

codecov-io commented Jul 31, 2016

Current coverage is 84.48% (diff: 100%)

Merging #4264 into 1.x will increase coverage by 0.02%

@@                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   

Powered by Codecov. Last update 3639e73...26ebeeb

* 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>
Copy link
Member

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.

@akarnokd
Copy link
Member

I'd have implemented a single operator for it but this will also do.

public class OperatorSortedTest {

@Test
public void testSortedList() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

testSorted

@akarnokd
Copy link
Member

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{
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: static

@akarnokd
Copy link
Member

akarnokd commented Aug 2, 2016

👍

1 similar comment
@davidmoten
Copy link
Collaborator

👍

@akarnokd akarnokd merged commit 0f23a15 into ReactiveX:1.x Aug 2, 2016
This was referenced Aug 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants