sockeqwe / mosby

A Model-View-Presenter / Model-View-Intent library for modern Android apps
http://hannesdorfmann.com/mosby/
Apache License 2.0
5.49k stars 841 forks source link

Mvi: Should use Relays, not Subjects #195

Closed ZakTaccardi closed 7 years ago

ZakTaccardi commented 7 years ago

Right?

sockeqwe commented 7 years ago

Back when I started working on this RxRelay2 wasnt available. However, there are two big disadvantages when switching to Relays (and the first one of them is a real blocker):

  1. RxRelay breaks the Observable contract for onComplete(). This is critical for MviBasePresenter.intent() wrapper function. Let's say that you have View that wants to fire a loadDataIntent() intent like this:
class FooActivity extends Activity {

    public Observable<Boolean> loadDataIntent(){
          return Observable.just(true); // This also completes , so onComplete() is called
   }
}

or something like this:

class FooActivity2 extends Activity {

    PublishSubject<Boolean>  loadData = PublishSubject.create();

    public Observable<Boolean> loadDataIntent(){
        return loadData;
    }

    public void onResume(){
        super.onResume();
        loadData.onNext(true);
       loadData.onComplete();
   }
}

If we would use RxRelay in MviBasePresenter.intent() then the Relay will simply ignore the onComplete terminal. Therefore, i.e. After a screen orientation change, the loadDataIntent() will fire again, although a onComplete() event has been sent before (but the internal relay ignored it). So with relays there is no way to complete a view's intent observable by sending a onComplete terminal event as specified by the Observable contract.

  1. While we could use BehaviorRelay internally for MviBasePresenter.subscribeViewState() I would like to avoid adding third party libraries. Just for this single subscribeViewState() method adding a third party library that we may (even if its unlikely) have to update / migrate and make a release of mosby once a third party library changes (in the case of breaking changes, I know, very unlikely) is something I want to avoid in general. BehaviorSubjects is doing the job (in subscribeViewState()) good enough in my opinion so that there is no major improvement in switching to BehaviorRelay.