RxTextView.textChanges emits mutable CharSequence

[jakewharton]

RxTextView.textChanges emits mutable CharSequence

[ZacSweers]

Are you suggesting that if the initial text is abc, and it's edited to say abcdef, then the emitted text should be def?

[artem-zinnatullin]

Are you suggesting that if the initial text is abc, and it's edited to say abcdef, then the emitted text should be def?

According to Delay Operator documentation — yes, because @simpleton delays the emission, not the subscription.

[erickok]

Are you not looking for throttle instead? Delay simply delays the emitting of every single item (event), not batch them.

[JakeWharton]

This is not an RxBinding issue. These operators are from RxJava. Please use the RxJava mailing list, its issue tracker, or StackOverflow with the rx-java tag.

[nsk-mironov]

The issue has nothing to do with RxJava. The real problem is that TextWatcher.onTextChanged invoked with the same mutable CharSequence instance, so typing a new character into EditText will make all delayed values invalid.

[JakeWharton]

.map(String::valueOf)? I'm not sure a defensive copy is the right default behavior.

[artem-zinnatullin]

+1 just checked delay in RxJava, it works according to documentation.

[JakeWharton]

I'll re-open for discussion around it, @nsk-mironov.

[artem-zinnatullin]

I think defensive copy of CharSequence is required because somebody may use observeOn() (or just multithreading in some operator) and do some async work with this CharSequence.

But the problem is that CharSequence is just an interface without copy(), though it's possible to do "copy" via subSequence(0, length() - 1), but some implementation of CharSequence may return non-fair copy…

I guess, for now, best RxBindings can do — is recommend (in javadoc) to map CharSequence to the String if user wants to do async processing.

[nsk-mironov]

I don't really think the issue needs to be fixed. There are some other operators that are affected by the same problem (e.g. ViewTouchOnSubscribe and ViewHoverOnSubscribe since both of them are using MotionEvent which can be recycled by android). Fixing TextViewTextOnSubscribe will imply fixing those operators as well.

The best thing we can do is explicitly mention in the documentation that these operators are not supposed to be used with other scheduling related operators.

[JakeWharton]

Yep, agreed. Will use this issue to track.

[artem-zinnatullin]

Since RxTextView.textChanges() looks like one of the most useful and popular binding, I think we should make it as safe as possible.

Mutability of the text from the user input may lead to a lot of hardly-discoverable problems in many apps (async processing, operators such as distinct, buffer, window and so on).

Proposal:
What if RxTextView.textChanges() will return String (-> unmodifiable).
For those who need CharSequence we will add RxTextView.rawTextChanges() with huge warning about the mutability of values in the javadoc.

MotionEvent is a special case, the fact that Framework has a pool of them and that MotionEvent is mutable described in the Documentation to the SDK, so it's fine for RxBindings too.
But far less developers know about mutability of CharSequence emitted by the TextView (I didn't know until @nsk-mironov commented here), with RxJava (async code as first-class citizen) + mutable values chances to shoot yourself in the foot are much higher!

[pakoito]

I, pakoito, being of sound mind and body, wholeheartedly agree with artem-zinnatullin proposal of desambiguation.

My reason is that I saw some misbehaviour with textChanges() + distinctUntilChanged() + other operators like debounce().

[valeriyo]

+1 to artem-zinnatullin's proposal. Observable of String would be highly desirable.