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

Base architectural problem #201

Closed orcchg closed 7 years ago

orcchg commented 7 years ago

Imagine we have a couple of classes:


Some basic classes:

interface SingleGridContract {
    interface View extends MvpView {
        void showSingleGrid();
    }

    interface Presenter extends MvpPresenter<View> {
        void onSingleGridClicked();
    }
}
class SingleGridActivity extends MvpActivity<SingleGridContract.View, SingleGridContract.Presenter> implements SingleGridContract.View {
    @Override
    public void showSingleGrid() { ... }
}
class SingleGridPresenter extends MvpBasePresenter<SingleGridContract.View> implements SingleGridContract.Presenter {
    @Override
    public void onSingleGridClicked() { ... }
}

All is OKAY here.


Now we decided to make a DERIVED presenter, because both will have lots of common business logic.

class MultiGridPresenter extends SingleGridPresenter implements ListGridContract.Presenter {
...
}
interface ListGridContract {
    interface View extends SingleGridContract.View {
       void showAdvanced();
    }

    interface Presenter extends SingleGridContract.Presenter {
       void advancedClick();
    }
}
class MultiGridActivity extends MvpActivity<### SingleGridContract.View ###, ListGridContract.Presenter> implements ListGridContract.View {
    ...
}

Note that this is impossible in current architecture to set proper View-type to MultiGridActivity, and the most we could do is to set parent type - SingleGridContract.View (compile error).

If we re-write sub-contract in the following way:

interface ListGridContract {
    interface View extends SingleGridContract.View {
       void showAdvanced();
    }

    interface Presenter extends MvpPresenter<View> {
        void advancedClick();
    }
}

in order to cope with proper View-type, we will get another compiler error while attempting to subclass SingleGridPresenter - now MultiGridActivity will try to inherit MvpPresenter with different types - MvpPresenter and MvpPresenter simultaneously and they clashes in Java (generics are not c++ templates =( ). I.e. it is impossible to call:

presenter.advancedCall(); from MultiGridActivity

So, out MultiGridActivity still could call it's advanced ListGridContract.Presenter methods, but we cannot call any advanced View-methods (they are not visible) from MultiGridPresenter, only basic methods from SingleGridContract.View (not from ListGridContract.View) are publicly visible and available. I.e. it is impossible to call:

getView().showAdvanced();  from MultiGridPresenter

Anyway, we can still safely cast the view:

((ListGridContract.View) getView()).showAdvanced(); // OKAY now

because ListGridContract.View is the actual View that attaches to the Presenter. But this is ugly run-time casting.

To be or not to be?

Thank You!

sockeqwe commented 7 years ago

I get your point but I'm not sure if this is a issue related to Mosby.

I think this is a general Java single inheritance and Generics issue or do you have a concrete solution / advice how to fix this issue in Mosby?

From my point of view the issue is:

interface Presenter extends SingleGridContract.Presenter 

and that SingleGridContract.Presenter is bound to SingleGridContract.View as described in the interface definition:

interface SingleGridContract.Presenter extends MvpPresenter<SingleGridView>

So yes, having something like this:

interface Presenter extends SingleGridContract.SingleGridPresenter,   MvpPresenter<ListGridContract.View> { }

would be the solution, however, this is not how java generics works because Generics are erased at compile time.

Again, I don't think that this is Mosby related.

Here are some thoughts how to prevent this from the very beginning: You said that basically your ListGridContract.Presenter extends from SingleGridContract.Presenter just because you want to share business logic. BusinessLogic is not meant to live in a Presenter. Extract Business Logic into a own Class. Then you can easily include that business logic in both, SingleGridContract.Presenter and ListGridContract.Presenter. Favor composition over inheritance. Mixins could solve single inheritance issue so that you don't have to extend a class, but rather add a interface to get a concrete implementation in. Unfortunately Mixins (interfaces with default methods) are only available with java 8 or Android API 24 + Jack. Kotlin supports Mixins (also prior Android API 24).

zourb commented 7 years ago

I faced the same problem, @orcchg , how did you solve this at last? thanks a lot.

orcchg commented 7 years ago

@zourb I've solved this the following way:

BaseActivity --> DerivedActivity --> UltimateActivity (inheritance)

interface UltimateContract {
    void foo();
}
UltimateActivity extends BaseActivity<**DerivedContract.View**, UltimateContract.Presenter> implements UltimateContract.View {

    @Override
    public void foo() {
    }
}
UltimatePresenter extends BasePresenter implements UltimateContract.Presenter {
   void bar() {
        ((UltimateContract.View) getView()).foo();
   }
}

So, I ve to cast. But this cast is logically safe.

zourb commented 7 years ago

I got it and i will have a try, thanks @orcchg