[Question] Where is the best place to start loading data and persist it?

[gcx-hci]

[Question] Where is the best place to start loading data and persist it?

[fzymek]

I've been able to find a solution to this problem, but is still feels like I am not using the library in a good way. I am calling loadData() from GalleryFragment#onResume() and I needed to change my presenter so it caches data/error and looks like this:

class GalleryPresenter extends TiPresenter<GalleryView> {

    private final static TiConfiguration PRESENTER_CONFIGURATION = new TiConfiguration.Builder()
            .setRetainPresenterEnabled(true)
            .build();

    private GettyImagesService service;
    private GettySearchResult result;
    private Throwable error;
    private boolean isRequestPending = false;

    GalleryPresenter() {
        super(PRESENTER_CONFIGURATION);
        Retrofit retrofit = new Retrofit.Builder()
                .addCallAdapterFactory(RxJavaCallAdapterFactory.create())
                .addConverterFactory(GsonConverterFactory.create())
                .baseUrl(Config.GETTYIMAGES_API_URL)
                .build();


        service = retrofit.create(GettyImagesService.class);
    }

    @Override
    protected void onCreate() {
        super.onCreate();
        Timber.d("onCreate");
    }

    @Override
    protected void onDestroy() {
        super.onDestroy();
        Timber.d("onDestroy");
    }

    @Override
    protected void onSleep() {
        super.onSleep();
        Timber.d("onSleep");
    }

    @Override
    protected void onWakeUp() {
        super.onWakeUp();
        Timber.d("onWakeUp");
        updateView();
    }

    public void loadData(String phrase, boolean pullToRefresh) {

        if (pullToRefresh) {
            clearData();
        }

        if (!hasData() && !isRequestPending) {
            Timber.d("loadData %s", phrase);
            getView().startLoading(pullToRefresh);
            isRequestPending = true;
            service.getImages(phrase)
                    .subscribeOn(Schedulers.newThread())
                    .observeOn(AndroidSchedulers.mainThread())
                    .subscribe(
                            result -> {
                                isRequestPending = false;
                                GalleryPresenter.this.result = result;
                                updateView();
                            },
                            error -> {
                                isRequestPending = false;
                                GalleryPresenter.this.error = error;
                                updateView();
                            }
                    );
        }
    }

    private void clearData() {
        error = null;
        result = null;
    }

    private boolean hasData() {
        boolean hasData = error != null || result != null;
        Timber.d("hasData %b", hasData);
        return hasData;
    }

    private void updateView() {
        Timber.d("updateView");
        if (isAwake()) {
            if (error != null) {
                Timber.d("showError");
                getView().showError(error);
            }
            if (result != null) {
                Timber.d("showGallery");
                getView().showGallery(result.getImages());
                getView().stopLoading();
            }
        }
    }
}

[passsy]

Please push this sample project somewhere so I'm able to pivot your code and reference parts of my solution in my answer

[fzymek]

@passsy You can find demo under https://github.com/fzymek/MVP-Sample

[passsy]

Hey Filip, finally found the time to update your sample project the ThirtyInch-way. Here is the updated code: https://github.com/passsy/MVP-Sample/blob/master/app/src/main/java/pl/fzymek/simplegallery/gallery/GalleryPresenter.java

Let me explain what I did:

First of all you shouldn't trigger loading from the Fragment (View). Never call something like loadData() from the View. The view should send events, and not tell the presenter what to do. Basically in your case there are two events when the data should be loaded. Initially and after a pull to refresh. The initial loading can be triggered by TiPresenter#onCreate() and the pull to refresh event can be simply forwarded to the presenter (presenter.onRefresh()).

Both events trigger the loadData() method and udpate the loading state as well as the error and result information.

You already implemented a updateView() method which generally updates the view based on the data known in the presenter. I modified it a bit but the idea was correct. Additionally I added @DistinctUntilChanged to the showGallery(list) and showLoadingIndicator(bool) method which prevents too many similar calls to the UI.

Cancelling the running request is the last missing piece. I cancel it it onDestroy() in case the request isn't finished when the user exits the Activity. It continues loading the data over a configuration change. When the request returns while the view isn't attached all state is known by the presenter. Although updateView() was called it never updates the view due to the null check. When the Activity resumes the view gets attached and onWakeUp() gets called triggering updateView(). This updates the view with the new data from the request.

[rafipanoyan]

Never call something like loadData() from the View. The view should send events, and not tell the presenter what to do.

Can you elaborate on the reasons why we should follow this rule ? Because I can read the exact opposite of what you've said on several blog posts including this valuable one.

[passsy]

I don't see the relationship to PRESENTERS DON'T NEED LIFECYCLE EVENTS.
Let's say it this way: There are good and bad events which can be forwarded to the presenter. Lifecycle events are the bad ones. They don't express a user interaction. Don't tell the presenter what to do, tell what happened. presenter.onButtonClicked(): Unit instead of presenter.loadData(): SomeData

Here is the good data flow which works in 99.8% of all cases:

View -----(User event)---> Presenter
View <--(Data, Actions)--- Presenter

Keep in mind that calling the Presenter from the View should not return data. The presenter should decide when and if view should get data or when actions should be executed.

[rafipanoyan]

The relationship with the article is that you seem to recommand to have lifecycle event in the presenter

The initial loading can be triggered by TiPresenter#onCreate()

And something like

Never call something like loadData() from the View.

Seem to be the opposite of what the article says. I think the best way is to have a loadData() method in the presenter and to trigger it on any Android lifecycle event we would like to (and which makes sense of course).
I'm ok with you when you say that the presenter must no return data and should decide when to forward it to view.

[passsy]

TiPresenter#onCreate is not the same as Activity#onCreate. TiPresenter#onCreate is only called once when the Presenter is created. After a configuration change, when Activity#onCreate with savedInstanceState != null is called the TiPresenter#onCreate will not be trigger. Same for TiPresenter#onDestroy which is only called once when the Activity will be finished, not always when Activity#onDestroy will be called.