36

I'm experimenting with the Architecture Components, and I want to build a ViewModel for each item of a RecyclerView. I'm not sure if that is formally correct or I should stick with the "old way".

I have this adapter:

public class PostAdapter extends RecyclerView.Adapter<PostAdapter.PostViewHolder> {

    private List<Post> list;
    public static class PostViewHolder extends RecyclerView.ViewHolder{
        final ItemPostBinding binding;

        public PostViewHolder(ItemPostBinding binding){
            super(binding.getRoot());
            this.binding = binding;
        }
    }

    @Override
    public PostViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
        ItemPostBinding binding = DataBindingUtil
                .inflate(LayoutInflater.from(parent.getContext()), R.layout.item_post,
                        parent, false);


        return new PostViewHolder(binding, parent.getContext());
    }

    @Override
    public void onBindViewHolder(PostViewHolder holder, int position) {
        holder.binding.setPost(list.get(position));
        holder.binding.executePendingBindings();
    }

    @Override
    public int getItemCount() {
        return list == null ? 0 : list.size();
    }

    public void setList(List<Post> list){
        this.list = list;
        notifyDataSetChanged();
    }
}

which works fine but it's very basic. how do I update it so each item has it's own ViewModel associated? is that even possible?

EDIT: playing with it, I've tried to put in ViewModels the following way:

public class PostAdapter extends RecyclerView.Adapter<PostAdapter.PostViewHolder> {

    private List<Post> list;
    public static class PostViewHolder extends RecyclerView.ViewHolder{
        final ItemPostBinding binding;
        private final Context context;
        private GalleryItemViewModel viewModel;

        public PostViewHolder(ItemPostBinding binding, Context context){
            super(binding.getRoot());
            this.binding = binding;
            this.context = context;
        }

        public Context getContext(){
            return context;
        }

        public void setViewModel(GalleryItemViewModel viewModel){
            this.viewModel = viewModel;
            binding.setViewModel(viewModel);
        }
    }

    @Override
    public PostViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
        ItemPostBinding binding = DataBindingUtil
                .inflate(LayoutInflater.from(parent.getContext()), R.layout.item_post,
                        parent, false);


        return new PostViewHolder(binding, parent.getContext());
    }

    @Override
    public void onBindViewHolder(PostViewHolder holder, int position) {
        GalleryItemViewModel vm = ViewModelProviders.of((FragmentActivity) holder.getContext()).get(GalleryItemViewModel.class);
        vm.setPost(list.get(position));
        holder.setViewModel(vm);
    }

    @Override
    public int getItemCount() {
        return list == null ? 0 : list.size();
    }

    public void setList(List<Post> list){
        this.list = list;
        notifyDataSetChanged();
    }
}

it works but is that the correct way to do it?

John
  • 185
  • 2
  • 14
jack_the_beast
  • 1,838
  • 4
  • 34
  • 67
  • hello Jacopo , you can use adapter in different way.. – Enamul Haque Nov 23 '17 at 10:35
  • can you give me an example or a link to follow? – jack_the_beast Nov 23 '17 at 10:36
  • you can check the answer from bellow link https://stackoverflow.com/questions/47432835/displaying-data-in-a-recyclerview/47433409#47433409 – Enamul Haque Nov 23 '17 at 10:36
  • if you feel till problem then ask me... – Enamul Haque Nov 23 '17 at 10:37
  • I don't see how this can help me, the given example does not use either databinding or viewmodels – jack_the_beast Nov 23 '17 at 10:41
  • 9
    `ViewModelProviders.of((FragmentActivity) holder.getContext()).get(GalleryItemViewModel.class)` Wouldn't this return same `GalleryItemViewModel` object on subsequent calls? I guess you should use the overloaded method `get(@NonNull String key, @NonNull Class modelClass)` of `ViewModelProvider` and passing unique key to get unique viewModel per `onBindViewHolder` call. – muthuraj Feb 08 '18 at 19:14
  • The ViewModel is created only once for all ViewHolders because the `context` passed comes from the parent. That's why it is working wrong, isn't it? – shurrok Jul 04 '19 at 12:19

3 Answers3

10

Funny, but answer - This is correct way, should be accepted :) You can make some code clean-up and remove GalleryItemViewModel from PostViewHolder, because you are creating hard reference and not using it. Then dirrectly in onBindViewHolder() use it like holder.binding.setViewModel(vm);

This is a link with MVVM code example that can help you.

Artem Mostyaev
  • 3,874
  • 10
  • 53
  • 60
Stanislav Bondar
  • 6,056
  • 2
  • 34
  • 46
6

First of all correct implementation of ViewModel should be by extending android.arch.lifecycle.ViewModel. The examples that extending BaseObservable which makes ViewModel class a data class but it should be presentation class since it's replacing the presenter of MVP pattern.

Another thing is ViewModelProviders.of(context).get(Class.class) returns the same ViewModel for every call and it lets you to share same data between views.

Also, ViewModel class should not, or contain minimum classes from Android environment and should not keep any references to view classes since it may outlive the views.

In your second example you probably getting same ViewModel from Activity/Fragment using

public void setViewModel(GalleryItemViewModel viewModel){
            this.viewModel = viewModel;
            binding.setViewModel(viewModel);
}

Can you share layout file and how you implement this with ViewModel class?

Link for sample in accepted answer is not a correct example for MVVM and data binding.

ViewModel class from the link set as you second example:

public class CommentHeaderViewModel extends BaseObservable {

    private Context context;
    private Post post;

    public CommentHeaderViewModel(Context context, Post post) {
        this.context = context;
        this.post = post;
    }

    public String getCommentText() {
        return Html.fromHtml(post.text.trim()).toString();
    }

    public String getCommentAuthor() {
        return context.getResources().getString(R.string.text_comment_author, post.by);
    }

    public String getCommentDate() {
        return new PrettyTime().format(new Date(post.time * 1000));
    }

}

This is a data class, not an ViewModel class as architecture components page states, it also imports view classes which is bad for unit testing.

It's data binding + RecyclerView tutorial, correct naming should not be ..ViewModel for this class. Check out this tutorial for data class and binding it with RecyclerView.

Thracian
  • 43,021
  • 16
  • 133
  • 222
  • Will take a look! Thanks for now – jack_the_beast Jun 27 '18 at 10:44
  • 4
    ViewModel class shouldn't have a context reference. – Wood Aug 29 '18 at 06:55
  • 2
    I wrote that it shouldn't, please read carefully. _Also, ViewModel class should not, or contain minimum classes from Android environment and should not keep any references to view classes since it may outlive the views._ That snippet show how ViewModel is used before and should not be used now, it's data class, not currently used architectural class for ViewModel. – Thracian Aug 29 '18 at 09:36
4

Make sure you assign an unique identifier when getting the ViewModel, because under the hood the ViewModelProviders will give you the same instance otherwise

get(some unique id, GalleryItemViewModel.class);

So please try adding an id there, like below :

 GalleryItemViewModel vm = ViewModelProviders.of((FragmentActivity) holder.getContext()).get(**some unique id**, GalleryItemViewModel.class);
    vm.setPost(list.get(position));
    holder.setViewModel(vm);
Dan Riza
  • 397
  • 3
  • 11
  • Good point on passing in unique `key`, however it's not needed in this case because he calls the `vm::setPost` on every binding, which presumably re-assigns all fields. Although I'm not sure if the way he's doing it would cause any performance issue. – WSBT Aug 29 '19 at 17:35