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 - retry intents #214

Closed TobiasRe closed 7 years ago

TobiasRe commented 7 years ago

I'm using the Mosby MVI library for a demo app that performs simple CRUD operations on a Restful API using Retrofit2 in my Interactor. I constructed the ViewStates very similar as in the sample App, containing Throwable when Retrofit responds with an error. In this case the render() function displays an error screen with a retry button. When the retry button is clicked I want to re-emit the last item for the Intent that failed.

I would like to do add something like retryWhen() to the Intent, which of course is only triggered by onError()

Is there a simple way of adding this kind of behavior?

sockeqwe commented 7 years ago

Hm, haven't thought about it too much yet, but I could think of the following solution: Model your error ViewState so that it contains all required data to retry. I.e. let's say that your Interactor needs some kind of id that is also required when retrying (by clicking on the retry button). Then your ViewState for error must also contain this id. Something like this:

class MyViewState {
     String id;
     Throwable error;
     // ...
}

The rest really depends on how you prefer to model your intents. i.e. you could have two intents like this:

interface MyView {
  Observable<String>  loadDataIntent(); // Fires in activity.onResume() for example
  Observable<String> retryLoadingIntent() // Fires when retry button clicked
}

class MyPresenter extends MviBasePresenter {

    protected void bindIntents(){
          Observable<String> loadIntent = intent(MyView::loadIntent);
          Observable<String> retryIntent = intent(MyView::retryLoadingIntent);
          Observable<String> merged = Observable.merge(loaodingIntent, retryIntent);

        Observable<MyViewState> vs = merged.flatMap( 
                                                             id -> interactor.load(id)
                                                                     .onErrorRetrurn( error -> new MyViewState(error, id) )
                                                                    ...
          );

       subscribeViewState(vs, MyView::render);
   }
}

Or you could already do the merging of both observables in the View Layer like this:

class MyViewAcitiyt extends MviActivity<MyView, MyViewState> implements MyView {

    PublishSubject<String> loadSubject =  ... ; // i.e. fires in onResume()
    PublisSubject<String> retrySubject =  ... ; // fires on retry Button click

   ...

   @Override
  public Observable<String> loadIntent(){
      return Observable.merge(loadSubject, retrySubject);
  }
}

I tend to prefer the first option because it is more explicit about the existence of a retry mechanism: I just have to look at the view interface to know what the view can actually do. It's just a matter of personal preference I guess.

P.S. Docs for merge() operator can be found here http://reactivex.io/documentation/operators/merge.html

btw. could you please post this question on stackoverflow too (and any further questions)? Because github is really bad to be indexed by search engines like google. I'm sure other will have this question too soon or later. Definitely more discoverable if it is put on stackoverflow (There is a Mosby tag on github).

TobiasRe commented 7 years ago

Thanks @sockeqwe ,

actually I resolved the issue using combineLatest very similar to your last approach, with the difference that the retrySubject doesn't need to know the content of the previous intent.

class MyViewAcitiyt extends MviActivity<MyView, MyViewState> implements MyView {
    PublishSubject<Boolean> retrySubject =  ... ; // fires on retry Button click
   ...

   @Override
  public Observable<String> searchIntent(){
      Observable<String> searchObservable = RxSearchView.queryTextChanges(searchView);
      return Observable.combineLatest(searchObservable, retrySubject, (searchString, retryClicked) ->     searchString);
  }
}

There is one thing to notice. combineLatest() only emits items when all Observables emitted a first item after subscribing to them. Thats why I need to initialize the retry after Mosby did its subscriptions.

@Override
  protected void onStart() {
    super.onStart();
    retrySubject.onNext(true);
  }

I will share this and additional questions like this on StackOverflow in the future. I raised this question, because it took me some time to solve and I'm pretty sure I won't be the only one with this feature in mind.