1.x: distinctUntilChanged change erroneous behavior #4276
Conversation
godardt
changed the title
| @@ -62,7 +62,7 @@ public OperatorDistinctUntilChanged(Func2<? super U, ? super U, Boolean> compara | |||
|
|
|||
| @Override | |||
| public Boolean call(U t1, U t2) { | |||
| return (t1 == t2 || (t1 != null && t1.equals(t2))); | |||
| return (t1 == t2 || (t2 != null && t2.equals(t1))); | |||
There was a problem hiding this comment.
Please add a test case that fails without fix and passed with it
There was a problem hiding this comment.
An example:
Object t1 and t2 are of the same type and contains an object x1.
Object x1 is null in t1 and is not null in t2
Object t1 and t2 override equals which in turn calls the equals of x1.
A NullpointerException is thrown from t1 but not from t2.
This is a basic example but what I want to say is that applying equals on t1 does not means that it will result the same the other way around.
That's the type of behavior changes which should be fixed on the user side of the rxjava lib but which can create confusion and can be avoided.
There was a problem hiding this comment.
The current comparison is completely legal, same as Java 7's Objects.equals(). Why did you swap the variables? Implementation of equals should be symmetric: a.equals(b) == b.equals(a).
There was a problem hiding this comment.
That's a broken equals implementation and not this library's problem.
|
Omg, I'm from phone at the moment, but it looks very critical, right? As far as I see it won't compare old value with new if old is |
Current coverage is 84.43% (diff: 100%)@@ 1.x #4276 diff @@
==========================================
Files 268 268
Lines 17476 17476
Methods 0 0
Messages 0 0
Branches 2664 2664
==========================================
- Hits 14768 14756 -12
- Misses 1854 1861 +7
- Partials 854 859 +5
|
|
ok, I'm with computer now and I don't see what's the problem with previous code. Can you please explain? |
|
@artem-zinnatullin My guess is that he has a bug in his |
|
@akarnokd Yes it's the case, it permitted me to pinpoint a bug on my side and I fixed it. But I though that if we could avoid people this behavior change it would be great |
|
After this change, all you need to do is reverse |
📘 PR summary
Fix behavior change of
distinctUntilChangedintroduced in the version1.1.7of RxJavaRelated to: #4034