kaushikgopal / RxJava-Android-Samples

Learning RxJava for Android by example
Apache License 2.0
7.55k stars 1.37k forks source link

fix: Combine latest example works after app pause -> resume #48

Closed marukami closed 7 years ago

marukami commented 8 years ago

The example now handles the app been paused and resumed.

kaushikgopal commented 8 years ago

Thinking out loud here (feel free to convince me otherwise):

i love the concept of adding a FormValidator and the check for .skipWhile(new FormIsClean()). However, i'm not a big fan of more abstraction.

The checks obviously make sense and should be added but is it possible to preserve the functionality by just more chaining and avoiding the abstraction?

I like having the long chaining because it's simpler to understand and observe the "Rx"ness part of the example without thinking too much about the logic that's happening.

marukami commented 8 years ago

So, I tried for a few hours to remove the need for a state object. First i tried creating two observable s, one for working out if the fields are dirty, the second for validating the form. But not matter how i structured the two observable s I would get the error "Invalid index 1, size is 1" from android.widget.TextView.sendOnTextChanged. This error seems to only happen when you have multiple consumers. Apart from that, I'm not sure why this happens. I'll have to investigate how textChanges works and if it's or not somehow causing this.

I did try a few other methods, but the Invalid index error would always come up.

So in the end I just refactored the code a little. I moved the validation logic out of the FormValidator and into the subscriber I also moved dirty check logic into the FormIsClean closure. I Then renamed FormValidator to Form since it now simple represents the forms input state at a given point in time. I this way is more Rx-ish and less abstract since it's still the consumers role to decide what to do with the state.

Let me know what you think?

kaushikgopal commented 7 years ago

replied to this in #57