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

Removing small amounts of MVI boilerplate #296

Closed
abyrnes opened this issue Dec 17, 2017 · 2 comments
Closed

Removing small amounts of MVI boilerplate #296

abyrnes opened this issue Dec 17, 2017 · 2 comments

Comments

@abyrnes
Copy link

abyrnes commented Dec 17, 2017

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
Copy link
Owner

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:
#209

Also, there are plans to add yet another way to implement MVI (MVVM-ish) where an MviView interface doesn't make sense:
#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
Copy link
Author

abyrnes commented Dec 17, 2017

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!

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

No branches or pull requests

2 participants