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.

Sign up for GitHub

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

Jump to bottom

1.x: distinctUntilChanged with direct value comparator - alternative #4034

Merged
merged 3 commits into from
Jun 20, 2016
Merged

1.x: distinctUntilChanged with direct value comparator - alternative #4034

merged 3 commits into from
Jun 20, 2016

Conversation

akarnokd
Copy link
Member

Alternative implementation to #4001.

@akarnokd akarnokd mentioned this pull request Jun 19, 2016
Closed
JakeWharton
JakeWharton reviewed Jun 19, 2016
View reviewed changes
src/main/java/rx/internal/operators/OperatorDistinctUntilChanged.java
return;
}

if (!comparison) {
Copy link
Contributor

@JakeWharton JakeWharton Jun 19, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

There's a subtle NPE potential here. Not sure if you want to explicitly guard against it from a misbehaving comparator implementation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch! I'll change the comparison to be primitive boolean to trigger NPE inside the try-block if necessary.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@JakeWharton
Copy link
Contributor

lgtm 👍

@akarnokd
Copy link
Member Author

Thanks Jake for the review!

@DavidMGross DavidMGross mentioned this pull request Jun 19, 2016
Open
@stevegury
Copy link
Member

👍

@akarnokd akarnokd merged commit 6bef4ce into ReactiveX:1.x Jun 20, 2016
@akarnokd akarnokd deleted the DistinctUntilChangedComparator branch June 20, 2016 17:43
@akarnokd akarnokd mentioned this pull request Jun 30, 2016
Closed
@EKami EKami mentioned this pull request Aug 3, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Feature-Request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@akarnokd @JakeWharton @stevegury