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

1.x: added distinctUntilChanged(comparator) #4001

Closed
wants to merge 3 commits into from
Closed

1.x: added distinctUntilChanged(comparator) #4001

wants to merge 3 commits into from

Conversation

Nexen23
Copy link

@Nexen23 Nexen23 commented Jun 9, 2016

For some cases, Observable.distinctUntilChanged(keySelector) isn't enough. For example, when data consists of 2+ objects with ID, you cannot simply summarize them and use it as a key. Also often there are stream of arrays of data which can't be distincted easily and generically.
So I think Observable.distinctUntilChanged(comparator) would be helpful. I use kinda tricky implementation, but think, it is okey. Firstly I tried to add ctor to OperatorDistinctUntilChanged(), but it was... too bad.

p.s. I was really confused by

If you would like to contribute code you can do so through GitHub by forking the repository and sending a pull request (on a branch other than master, 1.x, 2.x, or gh-pages).

and spent some time trying to create some sort of temporal branch here (newbie me). But finally looked up into PRs and so that it is okey to PR into 1.x. Is it an error in .md-file?

*/
public final Observable<T> distinctUntilChanged(Func2<? super T, ? super T, Boolean> comparator) {
return lift(new OperatorDistinctUntilChanged<>(new Func1<T, Object>() {
T previousValue;
Copy link
Member

@akarnokd akarnokd Jun 9, 2016

Choose a reason for hiding this comment

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

This will be shared among all subscribers of the chain and you won't get proper results when multiple subscribers run the chain concurrently.

@akarnokd
Copy link
Member

akarnokd commented Jun 9, 2016

Please add unit tests that verify the behavior.

* @see <a href="http://reactivex.io/documentation/operators/distinct.html">ReactiveX operators documentation: Distinct</a>
*/
public final Observable<T> distinctUntilChanged(Func2<? super T, ? super T, Boolean> comparator) {
return lift(new OperatorDistinctUntilChanged<>(new Func1<T, Object>() {
Copy link
Member

Choose a reason for hiding this comment

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

Our target is Java 6 where the diamond operator is not allowed.

@Nexen23
Copy link
Author

Nexen23 commented Jun 9, 2016

How about this one?

@akarnokd
Copy link
Member

akarnokd commented Jun 9, 2016

Sorry for the delay. I remembered there was a discussion about this overload a while ago in #395.

The verdict was to not have them and you have to create your own wrapper class with custom equals and use it as a keySelector.

The decision may be revised, depending on the other's opinions.

(Otherwise, I would have done it differently, not with that subscriberProvider).

@akarnokd akarnokd changed the title added distinctUntilChanged(comparator) 1.x: added distinctUntilChanged(comparator) Jun 19, 2016
@akarnokd
Copy link
Member

See #4034 about how I'd implement this instead (easier shown in code than as a comment). Let me know if you want to adjust your PR.

@Nexen23
Copy link
Author

Nexen23 commented Jun 19, 2016

By adjust your PR you mean copy-paste your implementation as successive commit to existed ones?

@akarnokd
Copy link
Member

By adjust your PR you mean copy-paste your implementation as successive commit to existed ones?

If you think that my PR doesn't cover everything you wanted then yes. Otherwise, I leave it to you.

@Nexen23
Copy link
Author

Nexen23 commented Jun 19, 2016

If you think that my PR doesn't cover everything you wanted then yes

No, I think your PR is just okay and do what I wanted and do it better than mine own 👍
So I guess I need to close this PR now?

@akarnokd
Copy link
Member

Yes, you can close this. Thanks for your contribution anyway! If you have any further improvement suggestions / PRs for the library don't hesitate to post them. (If you need some hints about operator coding styles and structure, try looking at the common and simpler operators implementations.)

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.

2 participants