Skip to content

Javadoc: explain that distinctUntilChanged requires non-mutating data to work as expected #6290

Closed

Description

@akarnokd

distinctUntilChanged keeps the last value/key (depending on the overload) so it can compare it agains the newer value/key via Object.equals. However, if the value/key is mutable and gets mutated in between elements, the operator may not work as expected and filter out the seemingly same data:

PublishSubject<List<Integer>> subject = PublishSubject.create();
List<Integer> list = new ArrayList<Integer>();
list.add(1);

subject.distinctUntilChanged().subscribe(System.out::println);

subject.onNext(list);
// prints [1]

list.add(2);

subject.onNext(list);
// does not print anything, but [1, 2] was expected.

In the example, the same reference is passed to distinctUntilChanged thus two subsequent items evaluate as same. This mistake by the user is also very common on Android and with text components using a mutable CharSequence.

I suggest adding a section to every distinctUntilChanged variant explaining the situation in short, something along the lines:

<p>
Note that if the element type of the flow is mutable, the comparison of the previous and current 
item may yield unexpected results if the items are mutated externally. Common cases are mutable
{@code CharSequence}s or {@code List}s where subsequent objects are actually the same 
references modified before/after {@code distinctUntilChanged}. It is recommended mutable data is
converted to an immutable one, such as `map(CharSequence::toString)` or 
`map(Collections::unmodifiableList)` for example, to avoid the situation.

Activity

  1. added this to the 2.2 backlog milestone on Nov 5, 2018
  2. ashishkrishnan commented on Nov 13, 2018

    @ashishkrishnan
    Contributor

    Nice. Never actually thought of this case. Very valid. 🎉

    Few questions before the PR.

    1. This should reflect on withLatestForm variants for Observable & Flowable types right?
    2. Additionally, will it be okay to link this issue to the doc for clarity?
  3. akarnokd commented on Nov 13, 2018

    @akarnokd
    MemberAuthor

    Hi.

    1. It's about the distinctUntilChanged where this mutation is most obviously affecting a flow. All overloads in both Observable and Flowable should be updated.
    2. We don't link to issues in the main javadoc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions