kaushikgopal / RxJava-Android-Samples

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

Can not subscribe to observable again once the app is unsubscribed with FormValidationCombineLatestFragment #46

Closed tomoima525 closed 7 years ago

tomoima525 commented 8 years ago

Hi,

First of all, thank you for the great samples, Kaushik. Very useful for RxJava beginners.

I have questions about the title. I found out that in Form Validation with CombineLatest sample (FormValidationCombineLatestFragment.java), observer can not subscribe to observable again once the app goes into background by pressing "HOME" button and then returns to foreground.

I thought this is because _subscription.unsubscribe() is call at onPause() and _combineLatestEvents() is called at onCreateView(). So, I changed the code as below so that observer are subscribed again when the app comes to foreground.

// FormValidationCombineLatestFragment.java
@Override
public void onResume() {
    super.onResume();
    _combineLatestEvents();
}

@Override
public void onPause() {
    super.onPause();
    if (_subscription != null) {
        _subscription.unsubscribe();
    }
}

However, this did not work. Then, I changed the code to call _subscription.unsubscribe() at onDestroy(), and finally it worked as I expected.

@Override
public void onDestroy() {
    super.onDestroy();
    if (_subscription != null) {
        _subscription.unsubscribe();
    }
}

So my question is,

  1. Is calling _subscription.unsubscribe() at onDestroy() a right decision?
  2. Why calling _combineLatestEvents() at onResume() can not re-subscribe to observable?

It would be helpful if anyone has solution for these questions. Thank you

marukami commented 8 years ago

1) The onDestroy is not not guaranteed to be called So I generally put my unsubscribe calls is the onStop. For this example onPause is also fine. 2) It does recreate the subscription and the observable s. The issues is when you restart the fragment you lose the observable s and the new observable once again will skip the first event. So, you have to enter something in each field again before it will validate the fields.

marukami commented 8 years ago

Sorry you are also right in that _combineLatestEvents should be in onStart or onResume.

tomoima525 commented 8 years ago

@marukami Thank you for your reply.

1) My concern here is that after the validation is passed and the app goes background and comes to foreground, the "Submit" button is kept enabled even if I changed inputs to invalid entries. The screen below shows the issue.

screen shot 2016-02-02 at 18 59 25

The only way to avoid this issue was to call _combineLatestEvents() at onCreateView() and _subscription.unsubscribe() at onDestroy(). Calling _subscription.unsubscribe() at onStop() reproduced the issue as well. I was wondering if there is any other option to solve this.

2)

new observable once again will skip the first event

Oh, I didn't know this function. I got your point. Thank you.

marukami commented 8 years ago

Instead of using skip you could use filter and prevent events from empty fields. So, that way when the fragment resumes form a valid state the Func3 is invoked, and editing any field will also invoke the Func3. I've made a fork you can see here CombineLastest with filter.

Only issue with doing it this way is you lose the invalid state when the field is empty. I'll work on dealing with that edge case after work.

marukami commented 8 years ago

So I added a skipWhile so it will now handle the empty field after editing use case. @kaushikgopal thoughts/suggestions on if this should be a PR?

tomoima525 commented 8 years ago

@marukami great. This is a better solution. I would recommend this as a PR.

kaushikgopal commented 7 years ago

hey @tomoima525 : sorry for keeping this issue alive forever. over the course of the numerous changes, at some point, i changed the unsubscribing to happen onDestroyView. This in concert with onCreateView should do the trick.

if i'm missing something, feel free to chime in.

as always thanks @marukami for the insight on what was happening in the first place.