Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Discussion]: Introducing MviBaseIOPresenter #247

Open
sockeqwe opened this issue May 29, 2017 · 7 comments
Open

[Feature Discussion]: Introducing MviBaseIOPresenter #247

sockeqwe opened this issue May 29, 2017 · 7 comments

Comments

@sockeqwe
Copy link
Owner

sockeqwe commented May 29, 2017

Maybe we should add another Presenter for MVI that comes closer to the idea what Jake Wharton presented here:

http://jakewharton.com/the-state-of-managing-state-with-rxjava/

The idea is to have an Input / Output Presenter something like:

/**
 * @param <I> The type of Input events (type of intents)
 * @param <O> The type of "Output" (type of ViewState)
 */
abstract class MviBaseIOPresenter<I , O> {
   
   PublishRelay input();
   BehaviorRelay output();

   abstract void bindIntent(); // called only once the view is attached the very first time
   void unbindIntent(); // called only once the view is destroyed permanently
}

Usage:

public class MyPresenter <InputEvent, MyViewState> {
  
   @Inject BackendApi backend; // i.e. some Retrofit interface

   @Override
   public void bindIntent(){
       input()
          .switchMap( event -> {
              if (event instanceof InputEvent.LoadingEvent)
                 return backend.loadFoo()
                     .map(MyViewState::content)
                     .startWith(MyViewState.loading())
                     .onErrorReturn(MyViewState::error);
             else  if (event instanceof InputEvent.PullToRefreshEvent)
                 return backend.loadFoo()
                     .map(MyViewState::content)
                     .startWith(MyViewState.pullToRefreshLoading())
                     .onErrorReturn(MyViewState::pullToRefreshError);
         })
        .subscribe(output);
   }

}
public class MyActivity extends MviActivity<MyPresenter> {

   private Disposable inputDisposable;
   private Disposable outputDisposable;
   @Inject MyPresenter presenter;

   @Override
   public MyPresenter createPresenter() {
      return presenter;
   }

   @Override
   public void onCreate(Bundle bundle){
            super.onCreate(bundle);

           // findViewById stuff and other view setup things 
           ...

        Observable <InputEvent> loadingEvent = Observable.just(new InputEvents.Loading());  // triggers loading immediately
        
        Observable<InputEvent> pullToRefreshEvent = RxSwipeRefreshLayout(swiperefreshlayout)
                                  .map( ignored -> new InputEvent.PullToRefreshEvent() )
                                  .subscribe(presenter.input());

         Observable<InputEvents> inputs = Observable.merge(loadingEvent, pullToRefreshEvent);
        
         inputDisposable =  inputs.subscribe(presenter.input());        
         outputDisposable = presenter.output.observeOn(AndroidSchedulers.main())
                                            .subscribe(this::render);
    }

   private void render(MyViewState state) {
       ...
   }

   @Override
   public void onDestroy(){
      super.onDestroy();
      inputDisposable.dispose()
      outputDisposable.dispose();
   }
}

This issue is just for brain storming and idea sharing.
Basically, Mosby will just be used to keep presenter through screen orientation changes. Since presenter.output is a BehaviorRelay latest "ViewState" will be automatically resubmitted after screen orientation change.
Lifecycle Management of Disposables must be done manually.

We could provide such a Presenter, although I personally like more the current approach of MviBasePresenter but you know ... Jake Wharton is a smart guy ... Maybe he (or someone else using Mosby) sees some advantages with this implementation ... it's definitely a little less "black magic" compared to MviBasePresenter.

Any feedback is very welcome!

@charbgr
Copy link
Contributor

charbgr commented Jun 1, 2017

I find it very strange if observables (like the above) are created on Activity/Fragment/View.

Observable<InputEvents> inputs = Observable.merge(loadingEvent, pullToRefreshEvent);

I can't think how you can write tests if you have observables with operators like .scan() on activities. Current approach seems more testable to me.

One of the most interesting part of the talk is the part with the Actions, transformers etc which can be useful for LCE. Τhis can lead Mosby for a new Redux module.

@charbgr
Copy link
Contributor

charbgr commented Jun 12, 2017

I thought it back yesterday. If you rename Input and Output to Source and Sink respectively, it reminds to me cycle.js, don't you think so? @sockeqwe

@sockeqwe
Copy link
Owner Author

Yes ... I thought naming it input / output is slightly easier to wrap your head around, but I'm definitely open for better naming suggestions. This is by no means a fixed API proposal.

However, I didn't wanted to make it to cyclish 😄 nor I want to introduce the concept of Drivers as cycle does. I'm not saying, that concepts of cycle like Drivers are not a good idea, I just don't want to go into the direction of a framework. I would rather just provide a simple "entry point" (kind of) and then its up to you how you want to implement your stuff, i.e. cyclish with drivers ...

@wrparrish
Copy link

👍 on the new presenter, and input / output seems straight forward to me.

@hussam789
Copy link

recently I came across Andre Staltz's presentation talking about StreamIO and MonadicIO here but he didn't mention why it's important to have a single input/output event stream:
image
Martin Fowler has an interesting article on his blog about Command Query Responsibility Segregation.
how about naming it MviStreamIOPresenter or MviCqrsPresenter?

@larten
Copy link

larten commented Nov 4, 2017

I think there is one good reason to implement this idea: We could leave the lot of boring PublishSubjects from the view (and interface of view) which are presents for every actions.
And actually we do this merge() before we pass the Observable to the subscribeViewState() method.

Oh and one more. As I wrote in an another issue, I believe it's can be necessary to unsubscribe from a few heavy resources when we navigate to another screen and rescubscribe on resume. With own lifecyclehandling it seems more easy to do this.

@sockeqwe With your example it seems more like MVVM, not MVP. Or not? Only the view knows the presenter, presenter doesn't need to have reference to the view.

"Basically, Mosby will just be used to keep presenter through screen orientation changes."
And I don't agree with this. I mean this is an awesome tool and knowledge or way to organise your code (presentation layer) and manage viewstates

@sockeqwe
Copy link
Owner Author

sockeqwe commented Nov 4, 2017

Thanks for your feedback!

We could leave the lot of boring PublishSubjects from the view (and interface of view) which are presents for every actions.

Not sure if I have understood that correctly but right now you can do the same:

interface MyView extends MvpView {
    Observable<Intent> intents()
    void render(MyState state)
}

Then you would have just one Observable where you pipe all your view Intent's through. Nobody said you have to define an "intent-method" for each intent.

As I wrote in an another issue, I believe it's can be necessary to unsubscribe from a few heavy resources and rescubscribe on resume

Not sure if that will solve that problem because MviBaseIOPresenter internally must have a BehaviorSubject for the latest state. Otherwise you can't keep the rx stream to your "business logic" alive if the view unsubscribes and resubscribes to presenter (i.e. during screen orientation changes). So this is not really solving your problem. To solve this problem you will have to tell the Presenter when to destroy the presenter and it's internal stream permanently. You can do that in Mosby by providing your own MviDelegate like ActivityMviDelegate. For example the default one that uses Mosby out of the box is ActivityMviDelegateImpl which does that in onStart() <--> onStop(). If you want to have something that operates onStart() <--> onPause() <--> onResume() <--> onStop() then you have to implement your own ActivityMviDelegate as this delegate is the component that is responsible to coordinate lifecycle of view and presenter. That is totally possible with MviBasePresenter right now but as you see, you would have to write the a Delegate for MviBaseIOPresenter too. So your described problem is independent from the Presenter implementation. It's all about coordinating lifecylce of both, view and presenter, and that is the responsibility of the MviDelegate.

With your example it seems more like MVVM, not MVP. Or not? Only the view knows the presenter, presenter doesn't need to have reference to the view.

Yes, that is correct.

Overall, yes, I will start to work on that soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants