moxy-community / Moxy

Moxy is MVP library for Android with incremental annotation processor and ktx features
MIT License
324 stars 33 forks source link

I update the list in the adapter, which is in the presenter. Bug or feature? #111

Closed noname-developer closed 4 years ago

noname-developer commented 4 years ago

In the presenter, I create a list, receiving data from the interactor, load it into the view through the "setListAdapter" method to add it to the adapter. And so I can update the list in the presenter and pull the "notifyNoteItemList" method from view to update it in the adapter. Is this normal behavior or am I doing something wrong?)

NotesPresenter:

public class NotesPresenter extends MvpPresenter<NotesMvp.view> implements NotesMvp.presenter {

List<Note> noteList;
   ........

    @Override
    public void onCreateView() {
        noteList = interactor.getNoteList();
        Collections.reverse(noteList);
        getViewState().setListAdapter(noteList);
        checkEmptyList();
    }

    @Override
    public void onResultNoteEditor(int result) {
        if (result == RESULT_OK) {
            interactor.getNote(interactor.getCurrentNote(), dataList -> {
                int index = getNoteItemIndex(dataList.get(0).getNoteID());

                switch (interactor.getNoteState()) {
                    case STATE_CREATE:
                        noteList.addAll(dataList);
                        break;
                    case STATE_EDIT:
                        noteList.set(index, dataList.get(0));
                        break;
                    case STATE_DELETE:
                        noteList.remove(index);
                        break;
                }
            });
            getViewState().notifyNoteItemList();
            checkEmptyList();
        }
    }

    private int getNoteItemIndex(int noteID) {
        Log.d("lol", "find note: " + noteID);
        for (int i = 0; i < noteList.size(); i++) {
            Note note = noteList.get(i);
            if (note.getNoteID() == noteID) {
                return i;
            }
        }
        return -1;
    }
}

NotesFragment:

public class NotesFragment extends MvpAppCompatFragment implements NotesMvp.view {

........

    @Override
    public void setListAdapter(List<Note> noteList) {
        this.noteList = noteList;
        Log.d("lol", "LIST SIZE: " + this.noteList.size());
        adapterNote = new NoteAdapter(getActivity(), noteList, presenter);
        recyclerView.setAdapter(adapterNote);
    }

    @Override
    public void notifyNoteItemList() {
        adapterNote.notifyDataSetChanged();
    }

    @Override
    public void onResultNoteEditor(int result) {
        presenter.onResultNoteEditor(result);
    }
........
}

It is important for me to keep the list in the presenter in order to find the desired item or find out the size of the list. Maybe it needs to be done somehow differently. I would be grateful for your help!

alaershov commented 4 years ago

Not sure what you mean by "bug or feature", but if you're asking for some advice on showing a list, here's some.

You'd better avoid using Adapter in Presenter, it's an implementation detail of View and the View should be responsible for updating the adapter. Instead, just store a List in Presenter, and show it on view: viewState.showList(list). You'll have much more simple and testable Presenter. The implementation of View can be something like this:

override fun showList(list: List<MyItem>) {
    adapter.submitList(list)
}

notifyDataSetChanged should happen inside the adapter, you can also use some existing adapter like ListAdapter, or use DiffUtils manually.

noname-developer commented 4 years ago

I am storing the list list in a presenter and passing it to the view for the adapter. But the bottom line is that I can change the list in the presenter, give the command to the view to notify the adapter, and it will somehow make the changes that were in the list in the presenter. The only time it won't work is if I pass getViewState.setListAdapter (new ArrayList<>(noteList)); to the view method. But if I pass the original list, the adapter will listen to it, although the list is in the presenter, I update the adapter through the view. I don't understand where the adapter has access to the list in the presenter, how does it catch changes in it? The presenter knows nothing about the adapter, it only passes the list to the view once. I hope I was able to convey the problem)

alaershov commented 4 years ago

 Well, you said it yourself, if you pass the original list instance, the adapter will hold on to it. When you make changes in the list, Adapter can see it because you gave it the list. In other words, the list in the adapter and the list in your presenter are the same object, because you pass the same reference. Check out the adapter source code to understand what's happening under the hood. It's a better practice to pass a copy of the list, this way you can do any changes to your internal list and update adapter only when you want.

noname-developer commented 4 years ago

Yes, I was already convinced of this)) I decided to continue making changes in the presenter list, there is always access to it, and in the view only notify the adapter about the changes. I sincerely hope this will not be a clumsy solution to this problem.

Presenter methods:

    @Override
    public void onUpdateNewNotes(List<Note> noteAddedList) {
        for (Note note : noteAddedList) {
                   noteList.add(note);
        }
        getViewState().notifyAddNoteItems(noteAddedList);
    }

    @Override
    public void onUpdateChangeNotes(List<Note> noteChangedList) {
        for (Note note : noteChangedList) {
            noteList.set(note.getOrder(), note);
        }
        getViewState().notifyChangeNoteItems(noteChangedList);
    }

    @Override
    public void onUpdateRemoveNotes(List<Note> noteRemovedList) {
        for (Note note : noteRemovedList) {
             noteList.remove(note.getOrder());
        }
         getViewState().notifyRemoveNoteItems(noteRemovedList);
    }

View methods:

@Override
    public void notifyAddNoteItems(List<Note> noteAddedList) {
        for (Note note : noteAddedList) {
            adapterNote.notifyItemInserted(note.getOrder());
        }
    }

    @Override
    public void notifyChangeNoteItems(List<Note> noteChangedList) {
        for (Note note : noteChangedList) {
            adapterNote.notifyItemChanged(note.getOrder());
        }
    }

    @Override
    public void notifyRemoveNoteItems(List<Note> noteRemovedList) {
        for (Note note : noteRemovedList) {
            adapterNote.notifyItemRemoved(note.getOrder());
        }
    }

In any case, thank you! I thought there was a problem with Moxy, but it looks like I still have a lot to understand in Android.

alaershov commented 4 years ago

Instead of writing all these methods manually you can just write single method showList(list) and let DiffUtil do all the work for you. It's better in terms of performance and you write much less code, try it out)

noname-developer commented 4 years ago

Already applied it in several places and it noticeably reduced my headache with the adapter. Please accept my thanks again!