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

Removing small amounts of MVI boilerplate #296

Closed abyrnes closed 6 years ago

abyrnes commented 6 years ago

Hi!

I've just recently started using Mosby/MVI within the past month and I have thoroughly enjoyed it so far. So first off I'd like to say thank you!

From the examples I've observed it seems there's some boilerplate that could be removed. All of the view interfaces have one thing in common: the render method. Is there a reason a method like this never made it into an interface? Perhaps one extending MvpView:

public interface MviView<VS> extends MvpView {
  void render(VS viewState);
}

Additionally, subscribeViewState seems to always be called at the end of bindIntents, so bindIntents could return the Observable that's passed as the first parameter to subscribeViewState and this call could instead be made internally. So my Presenter would look like the following:

public class MyPresenter extends MviBasePresenter<MyScreen, MyScreenViewState> {
  @Override protected Observable<MyScreenViewState> createViewStateObservable() {
    Observable<MyScreenViewState> obs1 = intent(...);
    Observable<MyScreenViewState> obs2 = intent(...);
    return Observable.merge(obs1, obs2);
  }
}

It doesn't remove tons of boilerplate code, but it removes enough where I think it's useful. Also, with this implementation, the mistake of forgetting to call subscribeViewState can't be made.

I'm sure there could be good reason why it hasn't been implemented this way. Any thoughts?

sockeqwe commented 6 years ago

Hey, thanks for your feedback! Indeed, there is some "boilerplate" if you implement MVI the way I am doing (and promoting) it. However, I would like to give the user of Mosby the freedom to use this library the way it works best for them and for example I don't want to limit them to use a MviView interface with a render(state) method.

We had this conversation some time ago: https://github.com/sockeqwe/mosby/issues/209

Also, there are plans to add yet another way to implement MVI (MVVM-ish) where an MviView interface doesn't make sense: https://github.com/sockeqwe/mosby/issues/247

So for now, I think it makes sense to let the developer (the user of this library) the choice how to implement MVI. Mosby should just offer some convenient classes around general problems like lifecycle and retaining presenters. I think it is not too much work for any developer who wants to use Mosby for MVI based applications to introduce a MviView and AbstractMviPresenter (which is what you have described as MyPresenter in your comment above) in their own app, without having to introduce it in Mosby.

What do you think?

abyrnes commented 6 years ago

Makes sense :)

Indeed it takes no time at all to extend from MviBasePresenter and create a base version of the implementation discussed above.

Thanks for the insight!